alewin / useWorker

⚛️ useWorker() - A React Hook for Blocking-Free Background Tasks
https://useworker.js.org/
MIT License
2.96k stars 99 forks source link

Bad isoworker import #105

Open jer-sen opened 3 years ago

jer-sen commented 3 years ago

[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 https://github.com/alewin/useWorker/blob/011d2a39be6c7e4113f81dc52c56a2b9ba8ed209/packages/useWorker/src/lib/createWorkerBlobUrl.ts#L1 At least until isoworker has a default export cf https://github.com/101arrowz/isoworker/issues/2

101arrowz commented 3 years ago

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

jer-sen commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 https://github.com/alewin/useWorker/pull/98#discussion_r559237704

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 commented 3 years ago

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 commented 3 years ago

BTW, consider trying this solution out: https://github.com/reactioncommerce/reaction-component-library/issues/399#issuecomment-467860022

rodrigonzalz commented 3 years ago

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

101arrowz commented 3 years ago

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.

conor909 commented 3 years ago

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