preactjs / wmr

👩‍🚀 The tiny all-in-one development tool for modern web apps.
https://wmr.dev/
MIT License
4.92k stars 109 forks source link

Package entry resolution fails when "browser" is an object #910

Open rschristian opened 2 years ago

rschristian commented 2 years ago

Brought up by https://github.com/preactjs/wmr/discussions/909

When a package's "browser" field is an object, resolve.exports will return the entire object, not a string file path:

When true and "browser" is an object, then legacy() will return the the entire "browser" object.

See resolve.exports#optionsbrowser

This presents an issue in packages like bn.js:

package.json

...
"browser": {
"buffer": false
},

https://github.com/preactjs/wmr/blob/9a6777ff186a288ce126d473013c41d1b8f33bc3/packages/wmr/src/plugins/npm-plugin/resolve.js#L10-L17

entry is { buffer: false }, not an import/file path. Besides the immediate issue of .replace not working, we'll need to fallback to "main" most likely.

danielweck commented 2 years ago

have you considered https://github.com/lukeed/resolve.exports ?

rschristian commented 2 years ago

Er, already what's used, per my comment above.

Not sure I follow?

danielweck commented 2 years ago

brain fart, sorry :)

kvanstee commented 2 years ago

Hi rschristian, how can this be resolved? My browser balks at bn.js. I don't fully understand the problem.

rschristian commented 2 years ago

@kvanstee Well ideally I think resolve.exports would return the "main" field when "browser" is formed as it is with bn.js (object, without an entry point), though I don't know what Luke thinks of that or if it's even feasibly correct.

I don't know if we correctly handle the "browser" spec as-is right now (not including this instance, which we definitely don't), so we could just strip it out or fallback to "main"/index.js in the case of it being an object.

kvanstee commented 2 years ago

@rschristian I notice a previous unresolved issue #784 mentions the same code that you do. Also there is a comment that a commit on npm-refactor2 solves the issue. Will the commit solve this issue as well (if it's not the same)? I have cloned npm-factor2 but can't get the yarn commands to compile the code. Do you have a wmr.cjs that does not break with bn.js?

rschristian commented 2 years ago

Ah yep, good find. This is indeed a duplicate.

Will the commit solve this issue as well (if it's not the same)?

Will the commit solve the issue? It might? I'd have to go digging through each commit to see which one did it, but that branch replaces the NPM plugin entirely. The change that corrects the issue is likely built upon a whole lot of other changes.

You can likely clone WMR's main branch and snip out 'browser' from the fields list to get a working build. Ah, nope. It relies upon buffer which WMR will not provide. Don't have a good answer for you at the moment, besides that you may want to look at other tools if you really need bn.js

bakeiro commented 2 years ago

Hi! I would like to use this library for my projects, but can't really do it with this bug... there is any update on this? :)

rschristian commented 2 years ago

@bakeiro There's no one actively working on WMR these days, no. Unless you'd like to submit a fix, it's unlikely to be resolved.