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

Package Export keys should be used in-order #1418

Closed
developit opened this issue Jan 7, 2021 · 2 comments
Closed

Package Export keys should be used in-order #1418

developit opened this issue Jan 7, 2021 · 2 comments

Comments

@developit
Copy link
Contributor

Describe the bug

Hiya! I'm going around digging into the Package Exports implementations in each bundler. As part of this, I noticed that Vite's handling of package.json "exports" field resolution isn't following the spec.

The spec indicates that the ordering of keys in "exports" objects defines their precedence, where the first matching key (direct string or via object traversal to matched nested key) is used.

At present, Vite uses a hard-coded list of keys that define precedence of object fields in "exports" (including nested objects):

if (typeof exp.browser === 'string') {
return exp.browser
} else if (typeof exp.import === 'string') {
return exp.import
} else if (typeof exp.default === 'string') {
return exp.default
}

Reproduction / Expected result

The above issue causes the following export map to be interpreted incorrectly:

{
  "exports": {
    "import": {
      "browser": "./browser.mjs"
    },
    "browser": "./browser.js",  // <-- Vite will resolve to this
    "default": "./fallback.umd.js"
  }
}

Vite will resolve the above package to ./browser.js rather than the expected ./browser.mjs.

Suggestions

I imagine this implementation comes from dealing with some packages shipping strange "exports" fields. I encountered this in WMR, and found a two-stage exports field resolution process that seems to address most of these cases. The first stage follows the spec approach but does not early-exit if resolved to require/default keys. Instead, those cases fall back to a second stage that uses fixed require/default resolution that tries to account for broken configurations. The code is pretty short and entirely self-contained, and I've been pondering publishing it as a standalone module (not sure if that's of interest!):
https://github.com/preactjs/wmr/blob/9fe81addf1ce4fc1117f5e059ef2d618a7e65b42/src/plugins/npm-plugin/resolve.js#L82-L125

@yyx990803
Copy link
Member

yyx990803 commented Jan 7, 2021

Thanks a lot! Refactored the resolve to be consistent with WMR.

I think it makes sense to have this as a standalone package - that way we can add more comprehensive test coverage for it and ensure behavior consistency across Vite and WMR. Maybe it can become a standard reference for packages shipping multi formats - since Node's documentation is confusing at best.

Looks like resolve isn't adding support for this any time soon.

@developit
Copy link
Contributor Author

Fantastic! You're alarmingly productive, haha.

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

No branches or pull requests

2 participants