refactor: Breaking changes to outputs & pkg.json #4652
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…, & use `.mjs` exclusively
rschristian
requested review from
developit,
marvinhagemeister and
JoviDeCroock
📊 Tachometer Benchmark ResultsSummaryduration
usedJSHeapSize
Resultscreate10kduration
usedJSHeapSize
filter-listduration
usedJSHeapSize
hydrate1kduration
usedJSHeapSize
many-updatesduration
usedJSHeapSize
replace1kduration
usedJSHeapSize
run-warmup-0
run-warmup-1
run-warmup-2
run-warmup-3
run-warmup-4
run-final
text-updateduration
usedJSHeapSize
tododuration
usedJSHeapSize
update10th1kduration
usedJSHeapSize
|
|
Size Change: -29.6 kB (-38.72%) 🎉 Total Size: 46.8 kB
ℹ️ View Unchanged
|
marvinhagemeister
approved these changes
Feb 11, 2025
|
Not sure why coveralls is breaking, weird. I don't think we'd gain much from |
|
fair point! |
rschristian
force-pushed
the
refactor/pkg-exports
branch
from
18935a3 to
ad213f4
Compare
JoviDeCroock
approved these changes
Feb 12, 2025
rschristian
force-pushed
the
refactor/pkg-exports
branch
2 times, most recently
from
a33b56f to
133776c
Compare
JoviDeCroock
pushed a commit
that referenced
this pull request
Feb 18, 2025
* refactor: Switch to `package.json#exports.module`, drop `.min` builds, & use `.mjs` exclusively * chore: Remove leftover CJS shell * test: Fix export for karma * fix: coverage not generated in minify tests --------- Co-authored-by: Marvin Hagemeister <[email protected]>
JoviDeCroock
pushed a commit
that referenced
this pull request
Feb 19, 2025
* refactor: Switch to `package.json#exports.module`, drop `.min` builds, & use `.mjs` exclusively * chore: Remove leftover CJS shell * test: Fix export for karma * fix: coverage not generated in minify tests --------- Co-authored-by: Marvin Hagemeister <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Lands a few breaking changes at once, happy to revert any part or all of this if we feel anything's a problem.
.module.jsfor.mjsObviously this is where the ecosystem is going and it already has pretty good support: Webpack started to adopt back in late 2018 and Node has supported it since v12.16 onward. This moves our effective back compat date up to about 5-6 years ago.
Perhaps too soon, but thought I'd at least suggest it.
package.json#exports.browsertopackage.json#exports.moduleThis comes up fairly often, usually due to test runners trying to load modules meant for the browser in Node and then crashing as our browser build does not adhere to Node's ESM semantics. See #4406, preactjs/jest-preset-preact#13 (comment), #3634. While I'd tend to call that ecosystem error, it's not going away any time soon and we don't even use this output; it's just a standard ESM build, nothing browser-specific.
Instead of dropping this entirely though we can switch to
modulewhich some bundlers use to guarantee the package isn't instantiated twice (Webpack's docs on the subject). We had this change in the v11 branch and I think it's good to carry over. Might also reduce some of the__Hreports we have to deal with..minbuildsThese largely appear to be broken as-is (
preact.min.module.jsis not ESM, there's no CJS build as the IIFE output overwrites it immediately,preact.min.umd.jsdoesn't appear to be valid UMD either); the only thing maybe of value is the IIFE outputpreact.min.js. It's 30b lighter thanpreact.umd.js, but I don't know who's actually using it (if anyone at all).Users are almost certainly better off with the plain UMD build and we'd be better off dropping it to reduce some confusion as it's unclear when this is meant to be used.