Pull Request Test Coverage Report forBuild 4622958383

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please seethese guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

• 79 of79(100.0%) changed or added relevant lines in10 files are covered.
• 1 unchanged line in1 file lost coverage.
• Overall coverage increased (+1.5%) to99.796%

Totals
Change from baseBuild 4504340943:	1.5%
Covered Lines:	641
Relevant Lines:	642

💛 -Coveralls

Just ran a quick check on 9.0.16 vs 10.0.beta, using Vite 4.2.1 +source-map-explorer:

• 9.0.16:immer/dist/immer.esm.mjs, 10.42KB
• 10.0.0-beta.4:immer/dist/immer.mjs, 15.78KB

If I hand-inspect the Vite bundle output, I don'tsee anyNODE_ENV references left in there, which is what I would expect - they exist in the ESM artifact, but the minifier should get rid of those.

If it helps at all, I just manually copied and pasted the minified versions of 9.0.16 and 10.0.0-beta.4 out of Vite-built bundles, ran them through Prettier, and am attaching them as a zip - maybe it'll help show where the differences are:

immer-minified-prettified.zip

Per theMap/Set thing: FWIW, we reallydon't want people using theMap/Set support with RTK because of serialization issues, so at least in our case that is a size increase we don't benefit from. But, I do understand how that can cause confusion and annoyance onyour side. (My own preference would be that it still be opt-in somehow if possible.)

Yeah, the two most immediate references are here:

• https://redux.js.org/style-guide/#do-not-put-non-serializable-values-in-state-or-actions
• https://redux.js.org/faq/organizing-state#can-i-put-functions-promises-or-other-non-serializable-items-in-my-store-state

We actually had an internal discussion aboutMap/Set usage again yesterday after seeing your comment, and concluded it's still not sufficiently worth it. Itis hypothetically possible to rework the Redux DevTools with one of the new "JSON superset" serialization libs, but it's fairly common for folks to want to persist state inlocalStorage (often using theredux-persist library), and that would run into serialization issues. There'd also be more nuance to try to explain in the docs, and it's simpler if we leave it at a blanket "Don't!" :)

@markerikson I reverted the MapSet opt-in in immer@10.0.0-beta.6, mind doing a check if it solves the bundle size increase?

@mweststrate yeah, I'll check on that tonight!

@mweststrate: Okay, 3 builds of the same RTKQ app, changing only the Immer version:

I'm still generally surprised that 10.x isn'tsmaller than 9.x, given the changes.

@mweststrate : well, you got me hooked :)

I cloned theimmer-10 branch and did some poking around. I think there's a few things going on:

• You're currently building the project with TSDX. That seems to be adding some polyfills, like_extends() (which you can see in the CJS dev build).
• That in turn is partly becausetsconfig.json is set totarget: "ES6".

So, those are adding up to a bunch of dead bytes that aren't needed.

Unrelated to the bundle sizes, there's also issues with the current build output in this beta:

• I know you've been flipping file names back and forth. Right now there's animmer.mjs bundle, but animmer.esm.js.map sourcemap file
• The sourcemaps themselves seem broken and incorrect. If you try loading some of them intohttps://evanw.github.io/source-map-visualization/ , you'll see that variables and segments don't line up correctly, making them semi-useless

FWIW, Ijust switchedredux andredux-thunk over to usehttps://tsup.egoist.dev/ . It's basically like TSDX conceptually, except that it's based on ESBuild + Rollup (and it seems to actually bemaintained atm).

I just tried making a local branch that switches over totsup instead, and after 15-ish minutes I've got a decent part of the output bundles generating. More than that, I've got the minified files shrinking noticeably. Inbeta.6,immer.cjs.production.min.js is 16542b, and withtsup I've already got it down to 13680b. I also tried generating a minified.mjs artifact for comparison, and that's down to 12941b.

Some other notes here:

• I see room for some byte shaving tricks. For example, I triedexport const getPrototypeOf = Object.getPrototypeOf and deleting theObject. in front of the uses, and that shaved off a few bytes because the variable name gets minified down to justE.
• I'm 95% set on dropping UMD bundles for Redux 5.0 / RTK 2.0, especially since you can make a pre-built ESM bundle that should be importable in-browser. Might want to consider that here.
• I'd say you can probably drop the Flow support too, really :)

Sooooo, with all that said:

If you're interested,I can try to do the rest of the TSDX ->tsup conversion, and file that as a PR against this branch.

Thoughts?

@markerikson thanks for pointing out tsup! I was already wondering what was the modern bundling setup since tsdx went unmaintained, which was also the reason I didn't want to poke too much in the config. So happy to receive a PR! Please do leave the flow types, as they're useful inside Meta, so I try to keep them roughly up to date :)

@mweststrate okay, sure! I'll try to put up a PR over the weekend.

@mweststrate done! See#1032 .

Please let me know if you have any questions or would like me to tweak things further!

@markerikson just publishedimmer@10.0.0-beta.7

🎉 This PR is included in version 10.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Yoursemantic-release bot 📦🚀