vazco / uniforms

A React library for building forms from any schema.
https://uniforms.tools
MIT License
1.96k stars 244 forks source link

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

Closed simplecommerce closed 3 years ago

simplecommerce commented 3 years ago

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

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

@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:

https://github.com/nodejs/node/issues/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 commented 3 years ago

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

radekmie commented 3 years ago

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

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

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

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