search

agilgur5 / mst-persist

Persist and hydrate MobX-state-tree stores (in < 100 LoC)
Other
87 stars 17 forks source link

When using AsyncLocalStorage, "Illegal invocation" error occurs #18

Closed Venryx closed 4 years ago

Venryx commented 4 years ago

My config is pretty simple:

persist('some', storeM, {
    blacklist: [],
}).then(() => { ... });

Yet I get this error:

VM1685:1 Uncaught TypeError: Illegal invocation
    at eval (eval at callWithPromise (mst-persist.esm.js:42), <anonymous>:1:1)
    at callWithPromise (mst-persist.esm.js:42)
    at Object.getItem (mst-persist.esm.js:26)
    at persist (mst-persist.esm.js:89)
    at RootUIWrapper.ComponentWillMount (Root.js:198)
    at RootUIWrapper.UNSAFE_componentWillMount (index.js:694)
    at callComponentWillMount (react-dom.development.js:13371)
    at mountClassInstance (react-dom.development.js:13461)
    at updateClassComponent (react-dom.development.js:16986)
    at beginWork$1 (react-dom.development.js:18505)

Googled it, and it appears to be the same issue here: https://stackoverflow.com/questions/41126149/using-a-shortcut-function-gives-me-a-illegal-invocation-error

To fix, you can either bind the getItem/setItem functions to the localStorage object, or do the equivalent by using a (...args)=>localStorage.XXX(...args) wrapper function.

For now I am fixing it by globally applying the bind:

window.localStorage.getItem = window.localStorage.getItem.bind(window.localStorage);
window.localStorage.setItem = window.localStorage.setItem.bind(window.localStorage);

However, the library should not require the developer to apply this global binding.

agilgur5 commented 4 years ago

Thanks for reporting this bug @Venryx ! Based on this and #17, you seem to be using several features that I and others don't and discovering bugs along the way -- thanks for working through these with us 🙂 Hopefully #4 will make these usage bugs happen less often

I have never seen this type of error either, but have noticed that some window functions rely on this context frequently. I'll have to look into it a bit more to understand what an optimal fix might look like, but my hunch is that I can use Function.call to get around this without changing too much; likely changing usage of callWithPromise to use .call instead of changing callWithPromise itself (it may be reused to support a generic sync adaptor in the future, but there's still more considerations for that and this issue adds to those).

It's good to know that there's a workaround in the meantime for anyone experiencing this, and that also means this isn't as high priority / doesn't need an immediate fix.

agilgur5 commented 4 years ago

So ironically I actually ran into this while adding tests in #21 😆 😅 . Using .call actually didn't work for some reason (not even as func.call(window, ...args)) and neither did .bind, so I just ended up using wrapper functions to fix this in #22 .

Will push out a new release very soon, especially as this (and #17 too due to addition of a test for it) are actually blocking #21 now, and that blocks #16 . Both #19 and #22 have been tested locally against #21, so can confidently release them now 🙂

agilgur5 commented 4 years ago

Fix has been released in v0.1.2