Follow up to #843, refining exports and browser field interaction #866
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jarreddebeer
force-pushed
the
browser
branch
2 times, most recently
from
1f43194
to
1f60002
Compare
shellscape
requested review from
LarsDenBakker,
guybedford,
lukastaegert and
danielgindi
|
It looks like we have some issues with pnpm and the latest major, and some dependency issues. I'll get those cleaned up soon. |
|
We've just updated |
added 5 commits
April 23, 2021 17:22
guybedford
reviewed
Apr 26, 2021
There was a problem hiding this comment.
Thanks for the PR @Jarreddebeer. I like the idea of having the browser: true option also enable the browser condition, so a +1 from me here.
packages/node-resolve/test/fixtures/node_modules/exports-browser/package.json
Outdated
Show resolved
Hide resolved
…rrides pkg.browser
guybedford
reviewed
Apr 27, 2021
guybedford
reviewed
Apr 27, 2021
guybedford
approved these changes
Apr 27, 2021
This was referenced Apr 30, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rollup Plugin Name:
node-resolveThis PR contains:
Are tests included?
Breaking Changes?
If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.
List any relevant issue numbers: #843, #786
Description
This is a follow up to #843 and #786 based on refining the interaction between the exports and browser field. Specifically of the cases tested in #843. It updates the resolution of the exports field to apply browser internal mappings from the browser field only. Update: It appears upstream changes to node-resolve/src/index.js handle the browser internal mappings, so I have removed the files changed relating to that from the PR.
To help aid usability of exports and browser interaction this PR also made it so that setting {browser: true} in the options will also automatically apply the browser exports conditions. This will avoid common pitfalls by having a single configuration for browser mapping.
Tests are adapted from #843 and the output are aligned with webpack output for the web target.
Thanks for your review in advance.