search

developit / microbundle

📦 Zero-configuration bundler for tiny modules.
https://npm.im/microbundle
MIT License
8.07k stars 363 forks source link

update rollup from 2.35.1 (2020-12-14) to at least 3.16.0 (2023-02-17), or ideally latest (4.6.1, 2023-11-30) #1067

Open 0xdevalias opened 11 months ago

0xdevalias commented 11 months ago

Splitting this off as a new issue so it can be tracked more easily. See the original issue for more context:

output.sourcemapIgnoreList was added to rollup in 3.16.0 (2023-02-17):

microbundle is using a super outdated version of rollup (2.35.1) from 2020-12-14:

So before we can implement this change, we'll need to upgrade to a modern version of rollup.

Originally posted by @0xdevalias in https://github.com/developit/microbundle/issues/1066#issuecomment-1837821773

microbundle is currently using rollup 2.35.1 (2020-12-14):

output.sourcemapIgnoreList (required to implement #1066) support was added in 3.16.0 (2023-02-17):

Latest version (at time of writing) is 4.6.1 (2023-11-30):

0xdevalias commented 11 months ago

Skimming the rollup CHANGELOG from 2.35.1 (2020-12-14) to 2.79.1 (2022-09-22, the last version before 3.x), there don't seem to be any obvious breaking changes:

3.0.0 (2022-10-11) lists a number of breaking changes that will need to be investigated deeper:

There don't appear to be any obvious breaking changes between 3.0.0 (2022-10-11) and 3.16.0 (2023-02-17):

There don't appear to be any obvious breaking changes between 3.16.0 (2023-02-17) and 3.29.4 (2023-09-28, the last version before 4.x):

4.0.0 (2023-10-05) lists a number of breaking changes that will need to be investigated deeper:

There don't appear to be any obvious breaking changes between 4.0.0 (2023-10-05) and 4.6.1 (2023-11-30):

@rschristian Can you have a quick look at the breaking changes in the 3.0.0 and 4.0.0 releases of rollup and advise whether you're happy for those with regards to microbundle (eg. specifically 'flow on' things like minimum required node version, etc).

If you're happy with those (or those up to a certain point), then I can look at what further changes will be required to bump the rollup version + related plugins/etc here.

rschristian commented 11 months ago

Eyeballing it, would say 3 is fine, 4 is probably not.

This might be a considerable amount of work -- depending on new defaults, might have to do a fair bit of tuning to keep module sizes down. Last I checked, 3 increased our outputs by a non-negligible amount.

output.sourcemapIgnoreList (required to implement https://github.com/developit/microbundle/issues/1066) support was added in 3.16.0 (2023-02-17):

Not sure this is the case ("required to implement"), should be doable with a dead-simple plugin, no need for upgrading.

Edit: Quick example:

function IgnoreListPlugin() {
    return {
        name: 'ignore-list-plugin',
        generateBundle(_opts, bundle) {
            const sourcemaps = Object.keys(bundle).filter((file) => /\.map$/.test(file));

            for (const file of sourcemaps) {
                const map = bundle[file];
                map.x_google_ignoreList = [ /* something */ ];
                bundle[file].source = JSON.stringify(map);
            }
        }
    }
}
daun commented 11 months ago

@rschristian Is there some way to make rollup use this type of custom plugin when bundling with microbundle?

rschristian commented 11 months ago

@daun Only by editing Microbundle's Rollup config directly.

0xdevalias commented 11 months ago

Eyeballing it, would say 3 is fine, 4 is probably not.

@rschristian Are you able to give some context on the 'why' of that? Is it the switch from XX to swc in 4.x that gives you doubts? Or something else?

This might be a considerable amount of work -- depending on new defaults, might have to do a fair bit of tuning to keep module sizes down. Last I checked, 3 increased our outputs by a non-negligible amount.

@rschristian Noted. I figure at the very least I can play around and add some info here that confirms that one way or the other.

My current thoughts on approach are to bump to latest 2.x + check that. Then try 3.x and check that (potentially both 3.0 and then latest 3.x), and then finally try 4.x; or at least find the point along that journey where required plugins/etc aren't able to work any more, etc.

Do you have a set of 'canonical repos' you test with microbundle for checking size changes/etc? I know preact, but would there be any others worth looking at as well, or is that 'good enough'?

I also see the following in tools/ which seem to relate to do some file size things; maybe that's the more canonical comparisons:

Not sure this is the case ("required to implement"), should be doable with a dead-simple plugin, no need for upgrading.

@rschristian Mm, fair enough. I guess I was thinking of it from a 'use the support in the tool' rather than 'roll your own' approach, but yeah, hacking it in via a plugin would probably work as well.

Since my mind is in this space though, I figure I'll at least do some initial recon on how big the 'modernise rollup version' work seems like it will end up being.

rschristian commented 11 months ago

Are you able to give some context on the 'why' of that?

I don't love bumping min Node to 18 so quickly.

Do you have a set of 'canonical repos' you test with microbundle for checking size changes/etc? I know preact, but would there be any others worth looking at as well, or is that 'good enough'?

Not really, I think our test suite here is usually a good way to check that. It could always be better of course, but it'll generally catch most issues.

I guess I was thinking of it from a 'use the support in the tool' rather than 'roll your own' approach

Long term, sure, but rolling our own will take a fraction of the time to land.

It's not exactly complicated functionality to stick an additional key into a sourcemap, after all.

0xdevalias commented 11 months ago

Looking at the dependencies in package.json, the following seem to relate to rollup plugins that may need version bumps alongside the main rollup package:

While it's OOS of this issue, there may also be potential benefits in bumping dependencies such as:

Also:

 console.warn
    Browserslist: caniuse-lite is outdated. Please run:
    npx browserslist@latest --update-db

    Why you should do it regularly:
    https://github.com/browserslist/browserslist#browsers-data-updating
0xdevalias commented 11 months ago

I don't love bumping min Node to 18 so quickly.

@rschristian nods, makes sense. Though 18 has already passed active support, and has about another 1yr 4mo left on security support; so while it's not required yet, it would make sense to move forward by the time security support ends IMO:

image

That said, at least a cursory skim seems to suggest that rollup 4.x may not be any faster/etc yet anyway:

Not really, I think our test suite here is usually a good way to check that.

@rschristian Cool, sounds good.

Currently builds itself as:

⇒ npm run build

> microbundle@0.15.1 build
> npm run -s build:babel && npm run -s build:self

Build "microbundle" to dist:
      13.5 kB: cli.js.gz
        12 kB: cli.js.br
      12.1 kB: microbundle.js.gz
      10.8 kB: microbundle.js.br
Build "microbundle" to dist:
      13.5 kB: cli.js.gz
        12 kB: cli.js.br
      12.1 kB: microbundle.js.gz
      10.8 kB: microbundle.js.br

Long term, sure, but rolling our own will take a fraction of the time to land.

It's not exactly complicated functionality to stick an additional key into a sourcemap, after all.

@rschristian nods, valid. I guess I tend to have a default to a perfectionist mindset of 'do it right' (which often leads to scope creep 😅)

0xdevalias commented 11 months ago

Hrmm.. getting a weird one while trying to test rollup 3.0.0 (+ related plugin bumps).

I'll keep looking, but have you ever run into this one before?

TypeError: brotliSize is not a function

It seems that this is coming from the babel transpiled code, where the import brotliSize from 'brotli-size'; is ending up with an object that contains a default key, not actually getting the default import itself.

Details ``` ⇒ npm run build > microbundle@0.15.1 build > npm run -s build:babel && npm run -s build:self Build "microbundle" to dist: 13 kB: cli.js.gz 11.5 kB: cli.js.br 11.6 kB: microbundle.js.gz 10.3 kB: microbundle.js.br TypeError: brotliSize is not a function at getSizeInfo (/Users/devalias/dev/0xdevalias/forks/microbundle/dist/cli.js:350:79) at /Users/devalias/dev/0xdevalias/forks/microbundle/dist/cli.js:1083:22 at Array.map () at Object.writeBundle (/Users/devalias/dev/0xdevalias/forks/microbundle/dist/cli.js:1078:64) at /Users/devalias/dev/0xdevalias/forks/microbundle/node_modules/rollup/dist/shared/rollup.js:1914:40 at async Promise.all (index 0) at async PluginDriver.hookParallel (/Users/devalias/dev/0xdevalias/forks/microbundle/node_modules/rollup/dist/shared/rollup.js:1842:9) at async /Users/devalias/dev/0xdevalias/forks/microbundle/node_modules/rollup/dist/shared/rollup.js:27239:13 at async catchUnfinishedHookActions (/Users/devalias/dev/0xdevalias/forks/microbundle/node_modules/rollup/dist/shared/rollup.js:26401:16) at async /Users/devalias/dev/0xdevalias/forks/microbundle/dist/cli.js:619:5 ``` ```diff diff --git a/package.json b/package.json index 554bebe..a4d7a25 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,8 @@ "scripts": { "build": "npm run -s build:babel && npm run -s build:self", "build:babel": "babel-node src/cli.js --target=node --format cjs src/{cli,index}.js", "build:self": "node dist/cli.js --target=node --format cjs src/{cli,index}.js", + "//build:self": "node --inspect-brk dist/cli.js --target=node --format cjs src/{cli,index}.js", "prepare": "npm run -s build", "prepare:babel": "babel src/*.js -d dist && npm t", "lint": "eslint src", @@ -78,12 +79,12 @@ "@babel/preset-env": "^7.12.11", "@babel/preset-flow": "^7.12.1", "@babel/preset-react": "^7.12.10", - "@rollup/plugin-alias": "^3.1.1", - "@rollup/plugin-babel": "^5.2.2", - "@rollup/plugin-commonjs": "^17.0.0", - "@rollup/plugin-json": "^4.1.0", - "@rollup/plugin-node-resolve": "^11.0.1", - "@surma/rollup-plugin-off-main-thread": "^2.2.2", + "@rollup/plugin-alias": "^5.1.0", + "@rollup/plugin-babel": "^6.0.4", + "@rollup/plugin-commonjs": "^25.0.7", + "@rollup/plugin-json": "^6.0.1", + "@rollup/plugin-node-resolve": "^15.2.3", + "@surma/rollup-plugin-off-main-thread": "^2.2.3", "asyncro": "^3.0.0", "autoprefixer": "^10.1.0", "babel-plugin-macros": "^3.0.1", @@ -99,12 +100,13 @@ "lodash.merge": "^4.6.2", "postcss": "^8.2.1", "pretty-bytes": "^5.4.1", - "rollup": "^2.79.1", + "resolve-from": "^5.0.0", + "rollup": "^3.0.0", "rollup-plugin-bundle-size": "^1.0.3", - "rollup-plugin-postcss": "^4.0.0", + "rollup-plugin-postcss": "^4.0.2", "rollup-plugin-terser": "^7.0.2", - "rollup-plugin-typescript2": "^0.32.0", - "rollup-plugin-visualizer": "^5.6.0", + "rollup-plugin-typescript2": "^0.36.0", + "rollup-plugin-visualizer": "^5.10.0", "sade": "^1.7.4", "terser": "^5.7.0", "tiny-glob": "^0.2.8", ```

Also noticed that after bumping rollup from 2.35.1 to 2.79.1, some changes like the following are showing in the test snapshots; haven't looked deeper into what causes that change yet:

● fixtures › build visualizer with microbundle

    expect(received).toMatchSnapshot()

    Snapshot name: `fixtures build visualizer with microbundle 3`

-   - Snapshot  - 1
+   + Received  + 1

-   - import a from"camelcase";var o=a("foo-bar");export default o;
+   + import a from"camelcase";var o=a("foo-bar");export{o as default};
      //# sourceMappingURL=visualizer.esm.mjs.map

Testing with the following command, and bumping just the version of rollup:

sed -i '' 's/"rollup": ".*"/"rollup": "^2.35.1"/' package.json && \
git checkout package-lock.json && \
rm -rf ./node_modules && \
rm -rf ./dist && \
npm install --ignore-scripts && \
npm test

# Or for a quicker run: npm test -- --testNamePattern 'build visualizer with microbundle'

I got the following results:

Presumably it's related to the tree-shaking change? (or something not explicitly mentioned in the CHANGELOG?)

So next up is figuring out how to update the relevant fixture snapshots for this so that the tests are less noisy for the next version bumps. Which it seems I can do with a combination of:

⇒ npm test -- --updateSnapshot
Failed Tests ``` FAIL test/index.test.js (110.847 s) fixtures ✓ build alias with microbundle (4311 ms) ✓ build alias-external with microbundle (1243 ms) ✓ build async-iife-ts with microbundle (5049 ms) ✓ build async-ts with microbundle (3659 ms) ✕ build basic with microbundle (1486 ms) ✓ build basic-babelrc with microbundle (1116 ms) ✕ build basic-compress-false with microbundle (170 ms) ✕ build basic-css with microbundle (3463 ms) ✕ build basic-dashed-external with microbundle (1505 ms) ✕ build basic-flow with microbundle (1062 ms) ✕ build basic-json with microbundle (870 ms) ✓ build basic-multi-source with microbundle (1619 ms) ✕ build basic-multi-source-css with microbundle (1715 ms) ✕ build basic-no-compress with microbundle (180 ms) ✕ build basic-no-pkg-main with microbundle (1037 ms) ✓ build basic-node-internals with microbundle (46 ms) ✕ build basic-ts with microbundle (3367 ms) ✕ build basic-tsx with microbundle (3123 ms) ✕ build basic-with-cwd with microbundle (1402 ms) ✕ build class-decorators-ts with microbundle (3618 ms) ✕ build class-properties with microbundle (1670 ms) ✕ build css-modules--false with microbundle (1002 ms) ✕ build css-modules--null with microbundle (1045 ms) ✕ build css-modules--string with microbundle (1300 ms) ✕ build css-modules--true with microbundle (1034 ms) ✓ build custom-babelrc with microbundle (1962 ms) ✕ build custom-outputs with microbundle (1327 ms) ✕ build custom-outputs-alt with microbundle (1533 ms) ✕ build custom-source with microbundle (954 ms) ✕ build custom-source-with-cwd with microbundle (915 ms) ✕ build default-named with microbundle (908 ms) ✓ build define with microbundle (996 ms) ✓ build define-expression with microbundle (348 ms) ✕ build esnext-ts with microbundle (5481 ms) ✕ build inline-source-map with microbundle (1287 ms) ✕ build jsx with microbundle (1167 ms) ✕ build macro with microbundle (1157 ms) ✕ build mangle-json-file with microbundle (1191 ms) ✕ build minify-config with microbundle (1290 ms) ✕ build minify-config-boolean with microbundle (1786 ms) ✕ build minify-path-config with microbundle (1402 ms) ✕ build minify-path-parent-dir-with-cwd with microbundle (1456 ms) ✕ build modern with microbundle (462 ms) ✕ build modern-generators with microbundle (1542 ms) ✕ build name-custom-amd with microbundle (1264 ms) ✕ build name-custom-cli with microbundle (1185 ms) ✕ build no-pkg with microbundle (1134 ms) ✕ build no-pkg-name with microbundle (1136 ms) ✓ build optional-chaining-ts with microbundle (3200 ms) ✓ build parameters-rest-closure with microbundle (1237 ms) ✓ build pretty with microbundle (1247 ms) ✓ build publish-config with microbundle (970 ms) ✓ build pure with microbundle (1073 ms) ✓ build raw with microbundle (1030 ms) ✓ build shebang with microbundle (1065 ms) ✕ build terser-annotations with microbundle (1118 ms) ✓ build ts-custom-declaration with microbundle (2883 ms) ✓ build ts-declaration with microbundle (2975 ms) ✓ build ts-jsx with microbundle (3023 ms) ✕ build ts-mixed-exports with microbundle (3694 ms) ✓ build ts-module with microbundle (3542 ms) ✕ build visualizer with microbundle (1170 ms) ✓ build worker-loader with microbundle (979 ms) ✓ should keep shebang (1 ms) ✓ should keep named and default export (5 ms) ```

After fixing the snapshots to account for the change from export default x to export {x as default} style that caused those test failures mentioned above; we can now try bumping all of the rollup plugins.

Diff ```diff diff --git a/package.json b/package.json index 554bebe..436dcd1 100644 --- a/package.json +++ b/package.json @@ -78,12 +78,12 @@ "@babel/preset-env": "^7.12.11", "@babel/preset-flow": "^7.12.1", "@babel/preset-react": "^7.12.10", - "@rollup/plugin-alias": "^3.1.1", - "@rollup/plugin-babel": "^5.2.2", - "@rollup/plugin-commonjs": "^17.0.0", - "@rollup/plugin-json": "^4.1.0", - "@rollup/plugin-node-resolve": "^11.0.1", - "@surma/rollup-plugin-off-main-thread": "^2.2.2", + "@rollup/plugin-alias": "^5.1.0", + "@rollup/plugin-babel": "^6.0.4", + "@rollup/plugin-commonjs": "^25.0.7", + "@rollup/plugin-json": "^6.0.1", + "@rollup/plugin-node-resolve": "^15.2.3", + "@surma/rollup-plugin-off-main-thread": "^2.2.3", "asyncro": "^3.0.0", "autoprefixer": "^10.1.0", "babel-plugin-macros": "^3.0.1", @@ -101,10 +101,10 @@ "pretty-bytes": "^5.4.1", "rollup": "^2.79.1", "rollup-plugin-bundle-size": "^1.0.3", - "rollup-plugin-postcss": "^4.0.0", + "rollup-plugin-postcss": "^4.0.2", "rollup-plugin-terser": "^7.0.2", - "rollup-plugin-typescript2": "^0.32.0", - "rollup-plugin-visualizer": "^5.6.0", + "rollup-plugin-typescript2": "^0.36.0", + "rollup-plugin-visualizer": "^5.10.0", "sade": "^1.7.4", "terser": "^5.7.0", "tiny-glob": "^0.2.8", ```

Which then leaves us with the following failing tests:

Failing Tests ``` ✕ build basic-css with microbundle (1453 ms) ✕ build basic-multi-source-css with microbundle (1888 ms) ✕ build class-decorators-ts with microbundle (778 ms) ✕ build css-modules--false with microbundle (1233 ms) ✕ build css-modules--null with microbundle (1195 ms) ✕ build css-modules--string with microbundle (998 ms) ✕ build css-modules--true with microbundle (956 ms) ✕ build custom-babelrc with microbundle (28 ms) ✕ build custom-outputs with microbundle (359 ms) ✕ build custom-outputs-alt with microbundle (389 ms) ✕ build modern with microbundle (18 ms) ✕ build modern-generators with microbundle (357 ms) ✕ build worker-loader with microbundle (21 ms) ```

Of which these can have their snapshots updated as they are just some basic CSS order changes/similar:

Easy Fixes ``` build basic-css with microbundle build basic-multi-source-css with microbundle build basic-multi-source-css with microbundle build css-modules--false with microbundle build css-modules--null with microbundle build css-modules--string with microbundle build css-modules--true with microbundle ```

But these ones I need to look into more still:

Requires further investigation ``` fixtures › build class-decorators-ts with microbundle src/index.ts:6:1 - error TS2354: This syntax requires an imported helper but module 'tslib' cannot be found. 6 @sealed fixtures › build custom-babelrc with microbundle BrowserslistError: [BABEL] /Users/devalias/dev/0xdevalias/forks/microbundle/test/fixtures/custom-babelrc/src/index.js: Unknown version 48 of op_mob (While processing: "/Users/devalias/dev/0xdevalias/forks/microbundle/node_modules/@babel/preset-env/lib/index.js") fixtures › build custom-outputs with microbundle BrowserslistError: [BABEL] /Users/devalias/dev/0xdevalias/forks/microbundle/test/fixtures/custom-outputs/src/index.js: Unknown version 48 of op_mob (While processing: "/Users/devalias/dev/0xdevalias/forks/microbundle/node_modules/@babel/preset-env/lib/index.js") fixtures › build custom-outputs-alt with microbundle BrowserslistError: [BABEL] /Users/devalias/dev/0xdevalias/forks/microbundle/test/fixtures/custom-outputs-alt/src/index.js: Unknown version 48 of op_mob (While processing: "/Users/devalias/dev/0xdevalias/forks/microbundle/node_modules/@babel/preset-env/lib/index.js") fixtures › build modern with microbundle BrowserslistError: [BABEL] /Users/devalias/dev/0xdevalias/forks/microbundle/test/fixtures/modern/src/index.js: Unknown version 48 of op_mob (While processing: "/Users/devalias/dev/0xdevalias/forks/microbundle/node_modules/@babel/preset-env/lib/index.js") fixtures › build modern-generators with microbundle BrowserslistError: [BABEL] /Users/devalias/dev/0xdevalias/forks/microbundle/test/fixtures/modern-generators/src/index.js: Unknown version 48 of op_mob (While processing: "/Users/devalias/dev/0xdevalias/forks/microbundle/node_modules/@babel/preset-env/lib/index.js") fixtures › build worker-loader with microbundle BrowserslistError: [BABEL] /Users/devalias/dev/0xdevalias/forks/microbundle/test/fixtures/worker-loader/src/index.js: Unknown version 48 of op_mob (While processing: "/Users/devalias/dev/0xdevalias/forks/microbundle/node_modules/@babel/preset-env/lib/index.js") ```

For those babel ones, there have been a LOT of changes since @babel/preset-env / etc v7.12.10; not sure if any of those are relevant to the error's we're seeing:

Also potentially tangentially relevant:

Might be related to autoprefixer and/or babel?

npm why browserslist

We should also have a closer look at the CHANGELOG for the following, and potentially try not bumping them fully to the latest versions:

TS2354: This syntax requires an imported helper but module 'tslib' cannot be found.

npm why tslib

You can see my exploratory WIP branch here:

rschristian commented 11 months ago

I guess I tend to have a default to a perfectionist mindset of 'do it right' (which often leads to scope creep 😅)

I'd hesitate to call that the "right" way -- it's the inbuilt way.

Supporting this behavior by plugin doesn't come with any negative repercussions and in fact it'll allow users to continue to use Microbundle on older Node versions. There isn't an inherent need to connect the feature you want with upgrading Rollup in this case.

It seems that this is coming from the babel transpiled code, where the import brotliSize from 'brotli-size'; is ending up with an object that contains a default key, not actually getting the default import itself.

Usually means something's up with the CJS plugin, yeah. Odd, nothing in brotli-size should be an issue from memory.

Also noticed that after bumping rollup from 2.35.1 to 2.79.1, some changes like the following are showing in the test snapshots; haven't looked deeper into what causes that change yet:

Terser, I believe. That's a better format, generally speaking, as more exports can be added with minimal size increase.

0xdevalias commented 11 months ago

I'd hesitate to call that the "right" way -- it's the inbuilt way.

There isn't an inherent need to connect the feature you want with upgrading Rollup in this case.

@rschristian Fair. I guess my brain has already forked from the original task, and so the 'right' that I was referring to here was more about updating the 2-3 year old build tooling to modern versions than it was about the simplest way to implement the original desired change.

Supporting this behavior by plugin doesn't come with any negative repercussions and in fact it'll allow users to continue to use Microbundle on older Node versions.

@rschristian Also fair. Though I guess at this stage, I'm somewhat interested to see what other benefits can come from modernising the build tooling versions.

Usually means something's up with the CJS plugin, yeah. Odd, nothing in brotli-size should be an issue from memory.

@rschristian Yeah, debugging + inspecting it it definitely seems to be a case of the transpiled code not accessing the default properly. Still not sure why that is though.

That's a better format, generally speaking, as more exports can be added with minimal size increase.

@rschristian Good to know. It increased the size of most of the test snapshots by a few bytes, but for the two 'more real world' looking ones, the sizes actually decreased by a few bytes.

I've been updating the test snapshots as I go (after reviewing that it makes sense to do so) on my WIP branch:

rschristian commented 11 months ago

I was referring to here was more about updating the 2-3 year old build tooling to modern versions

I'm somewhat interested to see what other benefits can come from modernising the build tooling versions.

I'm not sure there's going to be any improvements; Rollup hasn't changed much.

It increased the size of most of the test snapshots by a few bytes, but for the two 'more real world' looking ones, the sizes actually decreased by a few bytes.

From memory, I think they removed the logic to differentiate in these cases to instead just support the latter; the difference is small and the only ones adversely affected would be modules with a single default export, and only by a few bytes. Might not have been worth it on their end to support.