Fix exports map to support loading in Node esm

[fgnass]

Currently, preact-router fails to load in Node.js when it is imported from an ES module. Node correctly picks up the .module.js version as specified in the exports map, but loads it as cjs module because of the file extension. Here is a simple StackBlitz example that shows the current behavior.

This PR fixes the problem by changing the file extension to .mjs.

[rschristian]

Thanks for spotting this, really appreciate it!

[rschristian]

Could we revert this? It might be overly cautious, but there's no (functional) reason to subject these to Node's extension semantics, as Node never reads them. As unlikely as it is to be an issue, there are some older tools that will read "module" but will trip over .mjs, making this a breaking change.

A post-build cp to .mjs (if I'm remembering Microbundle's key preference right) would be sufficient.

[fgnass]

That's a good point! 👍 Another way to fix this without modifying the filenames at all would be to add "type": "module". Maybe this would be even safer? And we would not need to copy the files.

[rschristian]

"type": "module" would necessitate that all CJS entries switch to .cjs, which has even less support.

.mjs got some popularity and use a few years before .cjs & the current Node extension semantics were specc'd and solidified.

[fgnass]

Okay, you mean there are Node.js versions that support the "type" field but no exports maps? Or do you refer to bundlers that might also have adapted Node's file extension scheme but neither support the "module" field nor export maps? I had naively assumed, that all tools that know how to interprete the "type" field also support at least one way to specify CommonJS fallbacks. But I totally agree to play it safe here 👍 Ryan Christian ***@***.***> schrieb am Do., 26. Mai 2022, 00:07:

…

***@***.**** commented on this pull request. ------------------------------ In package.json <#425 (comment)> : > - "module": "dist/preact-router.module.js", - "jsnext:main": "dist/preact-router.module.js", + "module": "dist/preact-router.mjs", + "jsnext:main": "dist/preact-router.mjs", "type": "module" would necessitate that all CJS entries switch to .cjs, which has even less support. .mjs got some popularity and use a few years before .cjs & the current Node extension semantics were specc'd and solidified. — Reply to this email directly, view it on GitHub <#425 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAV22XA5W4YAUAMR3SOEKTVL2QCJANCNFSM5W2ADQ2A> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[rschristian]

There's a few odd Node versions that have some issues around "type" and export maps, but they're not really the issue.

The issue is that bundlers have consumed "module" and "main" for many years now, long before Node's .[cm]js semantics have existed. They may throw at seeing either of those extensions, as they predate them existing. react-native's Metro bundler is an example of such a tool, and just last week caused an issue when immer released a change containing "module": "<path>.mjs". Turns out Metro still has 0 support for .mjs: immerjs/immer#937

So it's best to keep those newer extensions locked behind "exports", as everything that can consume exports can handle those extensions just fine (or it's a bug if they cannot). For legacy fields, we have no guarantees that they'll support those extensions so we should try to keep them as .js to avoid breaking changes.

In this case, we add .mjs to "exports", keep all other ESM outputs as .js as Node's semantics do not need to apply to fields Node will never consume. Older versions of Node (out-of-date, mind you) will not consume "exports" and could throw at seeing .cjs for "main", so that too should be avoided (hence why "type": "module" would be bad).

[fgnass]

I updated the branch so that only the "module" exports use the .mjs extension. I used the copyfiles devDependency as it was already in place.

[rschristian]

Silly issue; somehow the test suite picks up and uses the new preact-router.mjs over the CJS version, even in a require() call. Just needed to specify the extension.

[rschristian]

Thanks for putting this together!

[fgnass]

It was a pleasure. Are there any plans for a new release on npm?

[lkmill]

@rschristian would be really nice if this could be published to npm... i currently cant upgrade my projects to "true" esm due to this.,..

[rschristian]

@lohfu Would probably recommend preact-iso over this anyways which does offer ESM, but I'll see if we can get a publish here.

[rschristian]

Jovi released v4.1.0 earlier today which includes this PR. Hope it helps!

[lkmill]

@rschristian cheers!