fix: Add export maps and .mjs to node ES modules (Fixes #10172) #10175
Conversation
🦋 Changeset detectedLatest commit: f54a03f The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Hi @rhengles, Welcome, and thank you for contributing to React Router! Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once. You may review the CLA and sign it by adding your name to contributors.yml. Once the CLA is signed, the If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at [email protected]. Thanks! - The Remix team |
rhengles
force-pushed
the
fix-vite-ssr-preact
branch
from
41667d9
to
91d6e19
Compare
This fixes an issue with Preact + Vite + SSR. Without the export maps, Vite SSR and Node.js gets confused and loads both the ESM and CJS versions of Preact. This breaks the hooks system of Preact as discussed here[1] and here[2]. During SSR, Vite starts as ESM and loads my modules (and Preact). But when I load React-Router, as it does not have the exports map, Node loads the CJS version, which loads the CJS version of Preact. Then Preact breaks because it can't find the hooks registered in the ESM code. Also, in the version of Node that I tested, v16.14.0, Node printed the warning below, so I also changed the extension of the ES module for node from ".js" to ".mjs". (node:5753) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension. [1] preactjs/preact#3220 (comment) [2] vitest-dev/vitest#747 (comment)
rhengles
force-pushed
the
fix-vite-ssr-preact
branch
from
91d6e19
to
f54a03f
Compare
|
This can't be done in a minor release. This is a major, breaking change. You can see our prior comments on this: #8268 (comment) I'm also not comfortable including such a change for a relatively esoteric build/library setup. If we're doing something major, it should be for a larger benefit. This really feels like something Vitest or Preact should fix. |
|
@timdorr Awesome, I wish that issue had been more discoverable, it would've saved an afternoon for me 😅 It is great that this is considered for v7. I appreciate the great effort that has been put into this library. Personally I can't see any downsides with this small change though. To me it clearly seems to align with the future of the Node/ESM ecosystem. Also Vite is many leagues ahead of webpack in every metric, you couldn't pay me to work with webpack. I respect this team's wishes and understand that I don't have the full picture. But I'll have to either monkey-patch it, fork it or switch to another router (one that I can use with React if required). Thankfully my app is still in the beginning... Beyond unforeseen consequences, I'd appreciate if you could share any known downsides. |
|
In theory, the same problem could happen with React + Vite SSR. I'll test that when I have time. Do you know if there is an example of this working out there ? |
|
Here is the fork if anyone else needs it https://www.npmjs.com/package/@arijs/react-router-dom Edit: After many test releases, I can happlily and proudly confirm that it is working!!! 🎉 🎉 🎉 |
This fixes an issue with Preact + Vite + SSR. Without the export maps, Vite SSR and Node.js gets confused and loads both the ESM and CJS versions of Preact. This breaks the hooks system of Preact as discussed here[1] and here[2].
During SSR, Vite starts as ESM and loads my modules (and Preact). But when I load React-Router, as it does not have the exports map, Node loads the CJS version, which loads the CJS version of Preact. Then Preact breaks because it can't find the hooks registered in the ESM code.
Also, in the version of Node that I tested, v16.14.0, Node printed the warning below, so I also changed the extension of the ES module for node from ".js" to ".mjs".
(node:5753) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.
[1] preactjs/preact#3220 (comment)
[2] vitest-dev/vitest#747 (comment)