fix: skip terser-webpack-plugin versions that leave bundles unminified - #1444
fix: skip terser-webpack-plugin versions that leave bundles unminified#1444giaBaoJS wants to merge 2 commits into
Conversation
terser-webpack-plugin 5.6.0 added per-minimizer asset filters and its terser implementation only accepts `.js`, `.cjs` and `.mjs` files. Re.Pack emits `.bundle` files, so every asset is filtered out before minification runs. No error or warning is reported and production bundles ship unminified. This affects both Rspack and webpack. Read the version of the resolved plugin and keep preferring the copy installed in the project root only while it can still minify Re.Pack's assets. Otherwise fall back to the copy shipped with Re.Pack and warn about the version that was skipped.
|
@giaBaoJS is attempting to deploy a commit to the Callstack Team on Vercel. A member of the Team first needs to authorize it. |
🦋 Changeset detectedLatest commit: ce4ccab The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The version check misses two cases. A plugin whose `package.json` is hidden behind an `exports` map reports no version and gets accepted even though it filters out `.bundle` assets, and a future release that starts accepting them would still be rejected because it is newer than 5.6.0. Load the resolved plugin and ask it directly: `terserMinify.filter` is what the plugin consults before minifying an asset, so a plugin is usable when it has no such filter or when the filter does not reject a `.bundle` name. Keep reading the version for the warning text only, and omit it from the message when it cannot be read.
|
I understand the desire to fix this but honestly my personal preference will be to fix the rspack minifier rather than continuing to default to terser |
makes sense 👍 since webpack is affected too, wdyt about keeping this fallback for webpack, but using/fixing native minimizer for rspack? |
|
but i believe terser still works for webpack, don't we already prefer the users version for that? |
|
I measured it before answering, and terser does not still work for webpack. Same root cause as Rspack. terser-webpack-plugin 5.6.0 added Real builds in
And preferring the user's version is what triggers this rather than what protects against it. webpack itself depends on Two things you may want before deciding on this PR:
new TerserPlugin({
test: /\.(js)?bundle(\?.*)?$/i,
extractComments: false,
minify: function repackTerserMinify(input, sourceMap, minimizerOptions, extractComments) {
return require('terser-webpack-plugin').terserMinify(
input, sourceMap, minimizerOptions, extractComments
);
},
terserOptions: { format: { comments: false } },
})I ran that with 5.6.1 in both the project and Re.Pack's own node_modules, so the default terser was a confirmed no-op and only the wrapper could do work: 1,969,176 bytes, byte identical to the 5.5.0 output. Happy to rewrite #1444 as that wrapper, or to close it if you would rather fix the Rspack minimizer first and handle webpack separately. |
|
I see, seems like i may have misunderstood the issue then. What do you propose as the right solution? I.e do you think what you mentioned in your comment (wrapper) is better or the solution presented in the pr already? |
|
The wrapper, and I would rather rewrite this PR as that than merge what is here now. What is here reacts to the symptom. It probes whether the installed plugin will refuse The wrapper removes the condition instead. In 5.6.1 the dispatch is if (typeof impl.filter !== "function" || ... impl.filter(name, info) !== false) {so the filter is only consulted when the configured The judgement call worth saying out loud: we would be deliberately stepping around a filter the plugin author added. I think it is defensible here, since the doc comment on that option describes it as "return true when the minimizer supports the asset" and terser genuinely does support this asset. Say the word and I will rewrite it. If you would rather land the Rspack minimizer switch first and treat webpack on its own, I am equally happy to close this and open the wrapper as a separate small PR whenever it suits you. |
Summary
terser-webpack-pluginand only prefer the copy installed in the project root while it can still minify Re.Pack's.bundleassetsgetMinimizerConfigunit tests covering the selectionWhy
terser-webpack-plugin5.6.0 added per-minimizer asset filters, and its terser implementation declaresfilter = (name) => /\.[cm]?js(\?.*)?$/i.test(name). Re.Pack emitsindex.bundleand[name].chunk.bundle, so every asset is rejected by the filter and dropped before minification runs. Nothing is reported: no error, no warning, and the asset is not flagged[minimized]in stats. Production bundles simply ship unminified.The report in #1390 attributes this to webpack internals missing under Rspack, but that is not the cause. Reproducing with the exact options Re.Pack passes, on a trivial entry, with only the plugin version changed:
minimizedminimizedSo webpack users are affected too, and the fix should not be scoped to Rspack.
Pinning
terser-webpack-pluginto 5.5.0 inpackages/repack/package.jsoncovers the fallback branch ofgetTerserPlugin, but not the root-first branch: a project that resolves 5.6.0 or newer at its own root still gets the silent no-op. That is the common case on pnpm and on hoisted layouts where another dependency pulls in a newer release.Implementation notes
The version gate is expressed against
terser-webpack-plugin, not against a bundler release, so it does not interact with the ongoing Rspack 2 work. When the version cannot be determined the plugin is assumed usable, which keeps the previous behaviour rather than failing a build over an unreadable manifest.Closes #1390.
Validation
pnpm --filter @callstack/repack test: 34 suites, 303 tests passedpnpm test: 10 tasks successfulpnpm typecheck,pnpm lint: cleangetMinimizerConfig.tswhile keeping the new tests turns the two selection tests red on the assertion (the project's incompatible plugin is chosen), not on an import or compile errorterser-webpack-plugin@5.6.1: before the change the asset is 1347 bytes and not minimized, after it is 210 bytes and minimized, with the fallback warning printed