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

[BUG]: Unable to resolve some modules (conflict with other package) #991

Closed
simplecommerce opened this issue Aug 3, 2021 · 7 comments · Fixed by #992
Closed

[BUG]: Unable to resolve some modules (conflict with other package) #991

simplecommerce opened this issue Aug 3, 2021 · 7 comments · Fixed by #992
Assignees
Labels
Type: Bug Bug reports and their fixes
Milestone

Comments

@simplecommerce
Copy link
Contributor

Hi,

I just noticed an issue with the latest version 3.5.2 when trying to use it with another package that I am using called threads.

https://github.com/andywer/threads.js

My project is running on MeteorJS and when I import uniforms in my App, and have the threads package installed, if I run my app.
I get this error:

Unable to resolve some modules:
  "internal/bootstrap/loaders" in /root/simple-todos-react/node_modules/esm/esm.js (web.browser)

If you notice problems related to these missing modules, consider running:
  meteor npm install --save internal

If I go back to 3.5.1 the App compiles and runs with no issue.
I am assuming something changed in the latest version?

@radekmie radekmie self-assigned this Aug 3, 2021
@radekmie radekmie added the Type: Question Questions and other discussions label Aug 3, 2021
@radekmie
Copy link
Contributor

radekmie commented Aug 3, 2021

Hi @simplecommerce. Yes, we introduced ESM build in #986 (it got released in 3.5.2) but I see no reason why it would cause such an issue. Could you create a minimal reproduction? Or is importing both uniforms and threads enough?

@simplecommerce
Copy link
Contributor Author

@radekmie It only seems to kick in when I import uniforms in a page with both packages installed.

From the research I did, its because threads has a dependency called [tiny-worker](https://github.com/avoidwork/tiny-worker) which depends on esm and inside esm.js the error that is getting thrown is related to this line.

https://github.com/standard-things/esm/blob/master/esm.js#L57

When I googled on the internet the error message I found this closed issue:

nodejs/node#33656

I just didn't know how to actually fix it, if its something that has to be modified in your library, threads or it would have to go all the way to esm ?

Here is the test I was using https://github.com/simplecommerce/bug-uniforms-threads-esm
You need to install meteor to make it work.
Then meteor run or npm start should work.
You will see the same error I wrote in the build process.

@radekmie
Copy link
Contributor

radekmie commented Aug 4, 2021

Thanks for the reproduction! I'm checking it right now.

@radekmie
Copy link
Contributor

radekmie commented Aug 4, 2021

Found it. The thing is, that we do specify both main and module in their "package paths", e.g., esm/index.js. It looks like Meteor does treat these as normal imports and not package-relative. Node.js docs do not specify what exactly should happen, though the comment on "using require()" may suggest it works as expected.

In any case, we'll fix that by prefixing these paths with ./. It'd be nice to verify how it works in Webpack and other bundlers and eventually file an issue for the Meteor bundler.

@radekmie
Copy link
Contributor

radekmie commented Aug 4, 2021

It looks like it's actually a bug in Meteor. These two comments 1, 2 suggest that these paths will be prefixed only if they were not found yet. In this case, they were, as the esm package was already scanned.

@simplecommerce
Copy link
Contributor Author

@radekmie Awesome, thanks for digging and finding the root issue, if you want you could create an issue on their repo and link them to your findings.

Thanks for your help and time I appreciate it!

@radekmie radekmie added Type: Bug Bug reports and their fixes and removed Type: Question Questions and other discussions labels Aug 4, 2021
@radekmie
Copy link
Contributor

radekmie commented Aug 4, 2021

@kestarumper Could you prepare a fix for this? Simply adding ./ in front of all these packages works just fine.

@radekmie radekmie added this to the v3.5 milestone Aug 4, 2021
@radekmie radekmie assigned kestarumper and unassigned radekmie Aug 4, 2021
@radekmie radekmie moved this to Closed in Open Source Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Bug reports and their fixes
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants