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

fix: sub-path module resolve wrong with pkg browser field #2001

Closed
wants to merge 7 commits into from

Conversation

ycjcl868
Copy link
Contributor

@ycjcl868 ycjcl868 commented Feb 8, 2022

Close #2002

@evanw
Copy link
Owner

evanw commented Feb 9, 2022

I just tried this out. Your PR does fix that issue. However, it breaks some test cases on https://github.com/evanw/package-json-browser-tests that were previously working, so we'll need to find a different fix that keeps everything working:

entry.js:
  require('foo')
package.json:
  { "browser": { "./foo": "./file" } }
file.js:
  input.works = true
entry.js:
  require('pkg')
node_modules/pkg/index.js:
  require('foo/bar')
node_modules/pkg/package.json:
  { "browser": { "./foo/bar": "./file" } }
node_modules/pkg/file.js:
  input.works = true
entry.js:
  require('pkg')
package.json:
  { "browser": { "pkg2": "pkg3" } }
node_modules/pkg/index.js:
  require('pkg2')
node_modules/pkg/package.json:
  { "browser": { "./pkg2": "./file" } }
node_modules/pkg/file.js:
  input.works = true

The https://github.com/evanw/package-json-browser-tests repo is where I'm have a collection of cross-tool tests for this feature. I'll add your tests from this PR to that repo too.

evanw added a commit to evanw/package-json-browser-tests that referenced this pull request Feb 9, 2022
@evanw evanw closed this in 25c6862 Feb 9, 2022
@ycjcl868
Copy link
Contributor Author

ycjcl868 commented Feb 9, 2022

I just tried this out. Your PR does fix that issue. However, it breaks some test cases on https://github.com/evanw/package-json-browser-tests that were previously working, so we'll need to find a different fix that keeps everything working:

entry.js:
  require('foo')
package.json:
  { "browser": { "./foo": "./file" } }
file.js:
  input.works = true
entry.js:
  require('pkg')
node_modules/pkg/index.js:
  require('foo/bar')
node_modules/pkg/package.json:
  { "browser": { "./foo/bar": "./file" } }
node_modules/pkg/file.js:
  input.works = true
entry.js:
  require('pkg')
package.json:
  { "browser": { "pkg2": "pkg3" } }
node_modules/pkg/index.js:
  require('pkg2')
node_modules/pkg/package.json:
  { "browser": { "./pkg2": "./file" } }
node_modules/pkg/file.js:
  input.works = true

The https://github.com/evanw/package-json-browser-tests repo is where I'm have a collection of cross-tool tests for this feature. I'll add your tests from this PR to that repo too.

Thanks!

@ycjcl868 ycjcl868 deleted the fix/browser-bug branch February 9, 2022 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resolve wrong path when import sub-path module with browser field
2 participants