Fix loss of local storage in Electron

[westbury]

What it does

As a result of #904 the window path name is prefixed to the local storage key. However in the Electron version this can break the persistence of local storage. This is because the window path name changes each time the program is run (when run in production mode). Therefore I suggest reverting to exclude pathname if running in Electron.

How to test

Run an Electron app on Linux. Change something that is persisted in local storage. Exit the app, closing down Electron and restart (do not just refresh). Run in production because in development the path name is the path to index.html for the desktop app.

Review checklist

• as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

[vince-fugnitto]

I'm not sure adding the dependency to core would be the way forward.
(the CI is failing because of it)

[spoenemann]

Wouldn't we rather bind a different implementation in the Electron Inversify module instead of checking at runtime?

[westbury]

Wouldn't we rather bind a different implementation in the Electron Inversify module instead of checking at runtime?

Good question, and that was how I was originally going to implement this. I chose to do it this way because:

1. There is precedent in testing at runtime, eg FileDownloadCommandContribution, and using Inversify seemed overkill for a single line of code.
2. I have no use-case for extenders needing to further customize the key prefix. I could simply rebind a class that extends StorageService and overriding prefix(). If that is what you have in mind then be aware that this could complicate things should extenders be rebinding StorageService for other purposes. So better would be to extract a separate interface and binding just for the prefix.

[kittaakos]

This is because the window path name changes each time the program is run (when run in production mode)

Nice catch!

Run an Electron app on Linux.

I have verified it on Windows but the window.location.pathname has the same effect.

👍 Thank you for the fix.

[thegecko]

@spoenemann Are you happy I merge this?

[spoenemann]

👍