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

Support module overrides in the browser field in package.json #1098

Closed
DimaLapshyn opened this issue Jan 28, 2022 · 6 comments
Closed

Support module overrides in the browser field in package.json #1098

DimaLapshyn opened this issue Jan 28, 2022 · 6 comments

Comments

@DimaLapshyn
Copy link

  • Rollup Plugin Name: plugin-node-resolve
  • Rollup Plugin Version: 13.1.1

Expected Behavior / Situation

If a package specifies module overrides in its "browser" field in package.json, e.g.

"browser": {
    "util": false,
    "worker_threads": false,
    "./errors": "./errors-browser.js",
    "./readable.js": "./readable-browser.js",
    "./lib/internal/streams/from.js": "./lib/internal/streams/from-browser.js",
    "./lib/internal/streams/stream.js": "./lib/internal/streams/stream-browser.js"
  }

the plugin import logic shall respect these overrides if { brower: true } has been supplied in the config (or if the browser field has been specified as the first priority via the mainFields option)

Actual Behavior / Situation

the plugin import logic seems to ignore such overrides, so they have to be monkey-patched with @rollup/plugin-replace.

Modification Proposal

Implement full support for the browser field, including support for module import overrides.

@guybedford
Copy link
Contributor

Browser field support is already implemented, so the question would be why this is not applying.

Do you have an exact replication here to share?

@frank-dspeed
Copy link
Contributor

frank-dspeed commented Feb 17, 2022

@guybedford i think improving the support for imports and exports should be more well defined so that when this filds are supplyed we take the browser fild from there we maybe should drop browser fild support in favor of that. typescript also introduced 2 new resolve algos to address that they call it node12 and nodenext if this is used the imports and exports get strict used.

the browser fild overlapps with the imports map implementation done by node

https://nodejs.org/api/packages.html#subpath-patterns

maybe i should reference this: microsoft/TypeScript#47931

@DimaLapshyn
Copy link
Author

DimaLapshyn commented Feb 17, 2022

@guybedford For example, given the overrides in the "browser" field here:
https://github.com/nodejs/readable-stream/blob/main/package.json#L56

The following require call does not seem to apply the overrides when processed by Rollup:
https://github.com/nodejs/readable-stream/blob/97fd94f404709542a7d69b271eddfcc34345916e/lib/_stream_readable.js#L1111

@stale
Copy link

stale bot commented Apr 24, 2022

Hey folks. This issue hasn't received any traction for 60 days, so we're going to close this for housekeeping. If this is still an ongoing issue, please do consider contributing a Pull Request to resolve it. Further discussion is always welcome even with the issue closed. If anything actionable is posted in the comments, we'll consider reopening it.

@stale stale bot closed this as completed Apr 24, 2022
@frank-dspeed
Copy link
Contributor

just my 5cent and a guide for people who come to this

Guide Browser & NodeJS & Everything Packaging

Create indipendent package.json files reference them as workspace in your package.json (root one) then install only the root one your done.

@shinichy
Copy link

shinichy commented Sep 7, 2023

I think this is still an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants