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

Add an option to whitelist certain packages as "can use the exports map directly" #14404

Closed
4 tasks done
JamesYeoman opened this issue Sep 18, 2023 · 7 comments · Fixed by #14723
Closed
4 tasks done

Comments

@JamesYeoman
Copy link

Description

According to this comment in the vite sourcecode, even if the exports map has a valid entry, it'll refuse to use it if it exists alongside the legacy fields, and if the exports map entry ends in .mjs.

Assuming this behaviour is still necessary for some packages, it still breaks for others.

For example: bwip-js

The exports map is thoroughly defined, but they need to define the legacy fields for backwards-compatibility for older codebases.

However, the legacy fields aren't sufficient enough to support ESM (as there's no legacy method for defining browser + esm + types), so they only support CJS via the legacy fields.

Packages like bwip-js should use the exports map where possible.

Suggested solution

In order to maintain the existing behaviour for packages that need it, there's two potential solutions

  1. Have a whitelist config option to whitelist packages that the developer knows has a suitable exports map
  2. Have a whitelist config option to whitelist packages that still have the "mjs files import cjs files" problem as defined in the vite sourcecode link

Both of these solutions would solve my particular use-case, but ultimately, the vite team would need to decide which is best for vite itself

Alternative

  • resolve.alias to manually specify the resolution path to node_modules/bwip-js/dist/bwip-js.mjs
    • Relies on relative paths, not ideal in a monorepo when importing source (have to define the alias to the node modules folder of the monorepo package that you're importing)
  • Yarn patch to rewrite the browser field in bwip-js
    • Hacky, requires storing the patch in the git repo, my team wasn't fond of that solution...

Additional context

No response

Validations

@bluwy
Copy link
Member

bluwy commented Sep 18, 2023

I don't think we're adding an option like this for the forseeable future, but we have also talked about removing the incorrect resolve altogether (.mjs specifically but could worth double-checking the rest). So maybe we can try that for Vite 5.

@JamesYeoman
Copy link
Author

I don't suppose the mjs resolution logic could be removed as a bugfix for v4, could it? Or is it being definitively classed as a breaking change?

@bluwy
Copy link
Member

bluwy commented Sep 19, 2023

Yeah it's classed as a breaking change as it's likely to cause small resolving issues for existing libraries. It's implemented as a heuristic so it works with some library, and removing it means causing some of these libraries to not work, so we can't do this in v4.

@JamesYeoman
Copy link
Author

Understandable. Is there an estimated timeline for v5?

@bluwy
Copy link
Member

bluwy commented Sep 20, 2023

Should be around mid-October, but we're also watching the next Rollup major release, so that may change slightly.

@JamesYeoman
Copy link
Author

@bluwy I had a look at the milestone for v5 but couldn't see anything that mentioned removing the exports map heuristic. Is this issue going to be addressed in v5?

@bluwy
Copy link
Member

bluwy commented Oct 19, 2023

Ah sorry about that. I forgot to add this to the milestone. We're close to the stable release now, but I'll put this on the milestone for now and will try to check if it works out for the ecosystem. It shouldn't be a big change.

@bluwy bluwy added this to the 5.0 milestone Oct 19, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Nov 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants