storage.getall() fails with jest-webextension-mock due to missing null argument

[shuntksh]

Hi Plasmo team,

Firstly, I want to thank you for your work on this library. Really impressive and an excellent DX so far!

I encountered a minor issue while using @plasmohq/storage with jest-webextension-mock. When using storage.getall(), the test fails as the underlying chrome.storage[area].get method is called without an argument, and that causes the jest-webextension-mock to throw with "Wrong key given."

According to the Chrome documentation, to retrieve the entire contents of storage, chrome.storage[area].get seem to expect a null as an argument to return all entries inside. However, the current implementation in @plasmohq/storage (source) calls the underlying method with no argument, which seems to work fine in today's Chrome implementation.

However, jest-webextension-mock expects chrome.storage[area].get to be called explicitly with null to return all values (source) which follows documented behavior. And this causes tests using @plasmohq/storage's storage.getall() to fail.

While both implementations seem correct, I am leaning towards an issue in the implementation of @plasmohq/storage, as it relies on Chrome's undocumented behavior. However, I understand this is not an issue for the library, too, as it works fine today.

As a workaround, I have written my mock to circumvent this issue by accepting an empty argument in chrome.storage[area].prototype.get. I am happy to open a bug report to the mock if you think this is a non-issue for the library or contribute to this library with fixes and tests.

Thanks!

[louisgv]

@shuntksh thank you so much for your kind words - PR is absolutely welcome :D. I think alignment with older API is definitely needed for a gradual upgrade. If possible, would love to have more tests and make the storage library more resilient across browser and extension runtime.

[shuntksh]

@louisgv, great to hear! While working on the fix, I noticed that there's a minor discrepancy in the type definition of @typed/chrome. I'm currently investigating it and will update this issue once it's resolved. However, as it doesn't seem to be causing any problems in this library, please feel free to close the issue if you'd like.

[louisgv]

I noticed that there's a minor discrepancy in the type definition of @typed/chrome

This is one of the gnarly part hahah - whether we should stick to the latest API, or add all the ts-ignore to polyfill with the deprecated API.

Another thought is to do something like this:

const oldChrome = globalThis.chrome as any

And we use oldChrome for some API that has type discrepancy with latest definition.