Replace microbundle with rollup#463
Conversation
372e7bb to
1bdcc40
Compare
33d630b to
5dd13c8
Compare
|
@addyosmani: WDYT about this PR? microbundle seems unmaintained. I have two TODOs I could use a second pair of eyes/second opinion. |
b608610 to
6f17536
Compare
f2c0577 to
070ad44
Compare
|
@addyosmani friendly ping :) |
There was a problem hiding this comment.
Thanks for tackling this @XhmikosR -agreed that microbundle being unmaintained makes this worth doing, and the rollup config is clean and well-structured.
I built both main and this branch side-by-side to check TODO #1, and the bundles are functionally equivalent: the core CJS still exports listen/prefetch/prerender, every output file is reproduced with the same name and format, and the only size movement is ~30–70 bytes gzipped on the core bundles (terser vs microbundle defaults), which is exactly what the .size-limit.json bump covers. dist/react/hoc.js is the one notable change - it's now run through terser instead of being emitted unminified by plain babel, so it drops from ~1.6 kB to ~0.6 kB gzipped with identical logic.
On TODO #2: I think we should skip sourcemaps and set sourcemap: false everywhere to match current behavior.
The config currently enables maps on every output, and since we ship files: ["dist"], that publishes them to npm - the unpacked tarball goes from ~70 kB / 12 files to ~249 kB / 21 files (+9 .map files), a ~3.5× jump.
The maps are self-contained so they're harmless functionally, but it's a meaningful weight increase for something nobody's asked for. Shipping sourcemaps is a reasonable thing to do, but I'd rather make it a deliberate, separate decision than fold it into a build-tool swap. Easy to add back later if we want them.
One minor non-blocker: terser's format.comments: false strips the Apache license header from the distributed files (it's currently retained in main's hoc.js). If we care about keeping license attribution in dist, we can add a banner in the rollup config or switch to comments: 'some'. Otherwise this is good to go once the sourcemaps are disabled.
|
Thanks for the review. Only issue left is the comments. I'll try to tackle it later. As for sourcemaps, I thought it'd be better for debugging, but we can enable them later if needed. |
|
So, about the comments, previously the react hoc.js was only passing through babel. Now it goes through the whole pipeline like everything else. No other dist file had comments and was already minified. So, the question is, do we want to keep it like that or like it was before? The current way is admittedly simpler. |
TODO: