KeithHenry / chromeExtensionAsync

Promise wrapper for the Chrome extension API so that it can be used with async/await rather than callbacks
MIT License
229 stars 32 forks source link

chrome.storage #7

Closed Thor99 closed 6 years ago

Thor99 commented 6 years ago

Hello! Congrats for the repo!

Will you add suppport to chrome.storage API?

qpongo commented 6 years ago

It works for me in both Chrome and Opera -- both use the "chrome" api.

It doesn't work in Edge which uses browser but doesn't seem to allow overriding of the methods (yet).

await browser.storage.local.set({key: 'value'});
let val = await browser.storage.local.get('key');
// val = {"key": "value"}
console.log(val.key);
// "value"
KeithHenry commented 6 years ago

@thor99 storage is in the collection of API wrapped in the async handlers, so it should work:

https://github.com/KeithHenry/chromeExtensionAsync/blob/8952e249e790a4034caaf82eea653873abf5e417/chrome-extension-async.js#L116-L117

https://github.com/KeithHenry/chromeExtensionAsync/blob/8952e249e790a4034caaf82eea653873abf5e417/chrome-extension-async.js#L207-L210

Are you having problems and could you provide more information about it if you are?

sag1v commented 6 years ago

actually i have a problem using storage with await, it returns undefined (the callback runs great though):

const cb = storage => console.log(storage);  // this is working good as a callback
//...
const storage= await chrome.storage.sync.get(storageKey, cb);
console.log(storage) // undefined printed
//...
KeithHenry commented 6 years ago

@sag1v I have a near identical call to const options = await chrome.storage.sync.get({ ... }) in a production Chrome add-in that's working fine. My guess would be that the API mapping call is either not running, or running in a context where it doesn't have access to the chrome.storage.sync instance yet.

Could you provide more information?

sag1v commented 6 years ago

@KeithHenry This is inside a react application as the extension itself (not content script or something similar). I'm sure the chrome.storage.sync is ready because when i use it with a callback it's working fine.

But Now when i think of it, I don't recall doing an import to your library only npm install.
I don't have access to the code at the moment so can't be sure, but if this is the case then sorry for wasting your time 😆 I guess i need to do import 'chrome-extension-async' in my entry point. I'll keep you update on this.

KeithHenry commented 6 years ago

@sag1v This needs to execute chrome-extension-async.js - that file contains an inline executing function that wraps the Chrome callback API with the async supporting one. It needs to run before you use the async syntax.

sag1v commented 6 years ago

@KeithHenry Ok, i can confirm that using the cdn in the entry point made it work (was my bad, i forgot to actually use the library! ).

I did find a different issue though with minification of the code as part of a build process. This is why i used the cdn instead of doing a normal: import 'chrome-extension-async'.

The thing is, I'm building the extension with react and using create-react-app` as the project template.

When i try to build it (while importing your lib) i get:

Failed to minify the code from this file: ./node_modules/chrome-extension-async/chrome-extension-async.js:13

It is because the way your code is written ES6 -> ES5 transpilation etc... You can read about why in here

Well, it is up to you if you want to support this.
If you do want, let me know and i'll open a new issue for this and try to help with a PR. :)

KeithHenry commented 6 years ago

@sag1v The intent of this module is to allow async/await keywords to be used in Chrome plug ins - as such I never considered that it would be used with ES5 transpilation. The only place this is going to run is in Chrome and that supports ES2018 features, never mind ES6.

Do you need to support versions of Chrome before 55 (when async/await came in) or 32 (when promises were added)? If that's the case the native callbacks may be a better pattern.

However, I would be interested in adding support for this scenario, I just wouldn't know where to begin testing it. PRs welcome if you have an idea what needs to change.

sag1v commented 6 years ago

@KeithHenry I'll open a new issue so we won't hijack this one.

8

KeithHenry commented 6 years ago

@Thor99 I've added clarification to the readme in the latest version (3.3.0) to clarify the need to import the scripts and for that to have run before using the async/await features.

If you're still having issues please re-open with additional details.