-
Notifications
You must be signed in to change notification settings - Fork 942
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Drop usage of chrome.storage
(or make the storage backend pluggable)
#476
Conversation
The browser version assumes that chrome.storage.local uses an API which is compatible with localStorage; which is not the case. Even though I am using chrome.storage.local for my Chrome extension, I would much prefer to keep debug's variable in localStorage, as I consider chrome.storage.local the 'private space' of my extension. This change obviates the need to support multiple storage types. But if storage type is important, how about supporting a custom storage facility, including chrome.storage.sync? I.e. the user would provide an object that follows the conventions. Just a thought - I certainly don't see the need at this point though.
I think the idea of using a "default" storage type and providing a way to "plug in" your own is a worthy idea. For now, though, this is a breaking change for existing extension support as the storage wouldn't be extension-specific. |
chrome.storage
(or make the storage backend pluggable)
I see your concerns. Also, chrome.storage.local requires a manifest.json permission. I find it rather surprising that any NPM module would avail itself of that facility without notifying me or without implementing it as an option. Therefore, IMHO, removing it and keeping localStorage / env variable as the built-in solution would be the best option. This would not be a breaking change, since chrome.storage.local support is already broken for those developing Chrome extensions and using chrome.storage. I suppose that having chrome.storage.local support would be useful for syncing debug behavior across websites and background, but personally I have no issues with manually adding the localStorage variable in 2 locations. localStorage also has the advantage of persisting across extension installs, which is not the case for chrome.storage.* Please see the Chrome Storage API reference to see how it's not localStorage-compatible: |
Please merge, you guys. Using |
tl;dr; Thank you for a great tool, we're always using it! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed - while this might have caused problems at one point this seems to be pretty ubiquitous now.
Thanks @EirikBirkeland! |
Will there be a release with this fix? |
I'd fork or pin package to Github commit :) |
The browser version assumes that chrome.storage.local uses an API which is compatible with localStorage; which is not the case. Even though I am using chrome.storage.local for my Chrome extension, I would much prefer to keep debug's variable in localStorage, as I consider chrome.storage.local the 'private space' of my extension.
This change obviates the need to support multiple storage types. But if storage type is important, how about supporting a custom storage facility, including chrome.storage.sync? I.e. the user would provide an object that follows the conventions. Just a thought - I certainly don't see the need at this point though.