Skip to content
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

Bad isoworker import #105

Open
jer-sen opened this issue Mar 20, 2021 · 10 comments · Fixed by #106 · May be fixed by #113
Open

Bad isoworker import #105

jer-sen opened this issue Mar 20, 2021 · 10 comments · Fixed by #106 · May be fixed by #113
Labels
bug Something isn't working

Comments

@jer-sen
Copy link

jer-sen commented Mar 20, 2021

[Required] Describe the bug
isoworker dependency does not provide a default import so isoworker.createContext raises an error from index.mjs.
I face this bug while using expo-camera bar code scanner on Web.

[Required] Expected behavior
No error.

isoworker should be imported as import * as isoworker from 'isoworker' here

import isoworker from 'isoworker'

At least until isoworker has a default export cf 101arrowz/isoworker#2

@101arrowz
Copy link

Yes, as I mentioned in the original PR, that needs to be import { createContext } from 'isoworker'.

@jer-sen
Copy link
Author

jer-sen commented Mar 21, 2021

Yes but I think it won't work without the full path. The solution should be:

import { createContext } from 'isoworker/esm/browser.js'

But it's an extraneous import so it's dangerous (if isoworker build change).
That's why I asked a default export to isoworker for a perfect long term solution.

@alewin
Copy link
Owner

alewin commented Mar 21, 2021

Yes but I think it won't work without the full path. The solution should be:

import { createContext } from 'isoworker/esm/browser.js'

But it's an extraneous import so it's dangerous (if isoworker build change).
That's why I asked a default export to isoworker for a perfect long term solution.

I agree, for now I am temporarily removing isoworker and the localdeps feature, I'll release the new version very soon

@101arrowz
Copy link

No, the pure isoworker import works. The "exports" field enables it to automatically find "esm/browser.js", and this should function with all bundlers (Rollup, Parcel, Webpack, etc.) I have tested this extensively and have done this before in 101arrowz/fflate, so I'm sure it works. At worst it needs some bundler config changed.

@jer-sen
Copy link
Author

jer-sen commented Mar 21, 2021

With from 'isoworker' esm/browser.js is found but since the import is not a precise file, only default export can be reached (which is undefined). Trying to import a specific export leads to error Can't import the named export 'createContext' from non EcmaScript module (only default export is available) cf #98 (comment)

With from 'isoworker/esm/browser.js' esm/browser.js is also found and since the import is a precise file, any export can be reached (especially createContext).

@101arrowz
Copy link

101arrowz commented Mar 21, 2021

I see, so it's an issue with the bundler config that is a bit difficult to change. In that case, does import { createContext } from 'fflate/browser' work? I'm guaranteeing that fflate/browser remains stable for browser imports, so if it works that's all that's necessary.

@101arrowz
Copy link

BTW, consider trying this solution out: reactioncommerce/reaction-component-library#399 (comment)

@rodrigonzalz
Copy link

When will this be fixed so we can get the local dependencies back?

@101arrowz
Copy link

If useWorker is configured to use the ESM export from isoworker, rather than letting the bundler decide to use the CommonJS one, it will work properly.

@alewin alewin linked a pull request Jun 15, 2021 that will close this issue
@conor909
Copy link

conor909 commented Jul 8, 2021

Is this still being worked on? Or is there a workaround?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants