fix: don't force terser on non-legacy (fix vitejs#6266) (vitejs#6272)

Author: agriffis

packages/playground/legacy/__tests__/legacy.spec.ts

@@ -1,4 +1,9 @@
-import { isBuild, readManifest, untilUpdated } from '../../testUtils'
+import {
+  findAssetFile,
+  isBuild,
+  readManifest,
+  untilUpdated
+} from '../../testUtils'
 
 test('should work', async () => {
   expect(await page.textContent('#app')).toMatch('Hello')
@@ -53,4 +58,19 @@ if (isBuild) {
       '../../../vite/legacy-polyfills'
     )
   })
+
+  test('should minify legacy chunks with terser', async () => {
+    // This is a ghetto heuristic, but terser output seems to reliably start
+    // with one of the following, and non-terser output (including unminified or
+    // ebuild-minified) does not!
+    const terserPatt = /^(?:!function|System.register)/
+
+    expect(findAssetFile(/chunk-async-legacy/)).toMatch(terserPatt)
+    expect(findAssetFile(/chunk-async\./)).not.toMatch(terserPatt)
+    expect(findAssetFile(/immutable-chunk-legacy/)).toMatch(terserPatt)
+    expect(findAssetFile(/immutable-chunk\./)).not.toMatch(terserPatt)
+    expect(findAssetFile(/index-legacy/)).toMatch(terserPatt)
+    expect(findAssetFile(/index\./)).not.toMatch(terserPatt)
+    expect(findAssetFile(/polyfills-legacy/)).toMatch(terserPatt)
+  })
 }

packages/plugin-legacy/index.js

@@ -105,23 +105,6 @@ function viteLegacyPlugin(options = {}) {
     name: 'vite:legacy-generate-polyfill-chunk',
     apply: 'build',
 
-    config() {
-      return {
-        build: {
-          minify: 'terser'
-        }
-      }
-    },
-
-    configResolved(config) {
-      if (!config.build.ssr && genLegacy && config.build.minify === 'esbuild') {
-        throw new Error(
-          `Can't use esbuild as the minifier when targeting legacy browsers ` +
-            `because esbuild minification is not legacy safe.`
-        )
-      }
-    },
-
     async generateBundle(opts, bundle) {
       if (config.build.ssr) {
         return
@@ -297,6 +280,11 @@ function viteLegacyPlugin(options = {}) {
       // legacy-unsafe code - e.g. rewriting object properties into shorthands
       opts.__vite_skip_esbuild__ = true
 
+      // @ts-ignore force terser for legacy chunks. This only takes effect if
+      // minification isn't disabled, because that leaves out the terser plugin
+      // entirely.
+      opts.__vite_force_terser__ = true
+
       const needPolyfills =
         options.polyfills !== false && !Array.isArray(options.polyfills)
 

packages/plugin-legacy/package.json

@@ -33,6 +33,6 @@
     "systemjs": "^6.11.0"
   },
   "peerDependencies": {
-    "vite": "^2.0.0"
+    "vite": "^2.7.8"
   }
 }

packages/vite/src/node/build.ts

@@ -365,7 +365,7 @@ export function resolveBuildPlugins(config: ResolvedConfig): {
     post: [
       buildImportAnalysisPlugin(config),
       buildEsbuildPlugin(config),
-      ...(options.minify === 'terser' ? [terserPlugin(config)] : []),
+      ...(options.minify ? [terserPlugin(config)] : []),
       ...(options.manifest ? [manifestPlugin(config)] : []),
       ...(options.ssrManifest ? [ssrManifestPlugin(config)] : []),
       buildReporterPlugin(config),

packages/vite/src/node/plugins/terser.ts

@@ -4,29 +4,46 @@ import type { Terser } from 'types/terser'
 import type { ResolvedConfig } from '..'
 
 export function terserPlugin(config: ResolvedConfig): Plugin {
-  const worker = new Worker(
-    (basedir: string, code: string, options: Terser.MinifyOptions) => {
-      // when vite is linked, the worker thread won't share the same resolve
-      // root with vite itself, so we have to pass in the basedir and resolve
-      // terser first.
-      // eslint-disable-next-line node/no-restricted-require
-      const terserPath = require.resolve('terser', {
-        paths: [basedir]
-      })
-      return require(terserPath).minify(code, options) as Terser.MinifyOutput
-    }
-  )
+  const makeWorker = () =>
+    new Worker(
+      (basedir: string, code: string, options: Terser.MinifyOptions) => {
+        // when vite is linked, the worker thread won't share the same resolve
+        // root with vite itself, so we have to pass in the basedir and resolve
+        // terser first.
+        // eslint-disable-next-line node/no-restricted-require
+        const terserPath = require.resolve('terser', {
+          paths: [basedir]
+        })
+        return require(terserPath).minify(code, options) as Terser.MinifyOutput
+      }
+    )
+
+  let worker: ReturnType<typeof makeWorker>
 
   return {
     name: 'vite:terser',
 
     async renderChunk(code, _chunk, outputOptions) {
+      // This plugin is included for any non-false value of config.build.minify,
+      // so that normal chunks can use the preferred minifier, and legacy chunks
+      // can use terser.
+      if (
+        config.build.minify !== 'terser' &&
+        // @ts-ignore injected by @vitejs/plugin-legacy
+        !outputOptions.__vite_force_terser__
+      ) {
+        return null
+      }
+
       // Do not minify ES lib output since that would remove pure annotations
-      // and break tree-shaking
+      // and break tree-shaking.
       if (config.build.lib && outputOptions.format === 'es') {
         return null
       }
 
+      // Lazy load worker.
+      worker ||= makeWorker()
+
       const res = await worker.run(__dirname, code, {
         safari10: true,
         ...config.build.terserOptions,
@@ -41,7 +58,7 @@ export function terserPlugin(config: ResolvedConfig): Plugin {
     },
 
     closeBundle() {
-      worker.stop()
+      worker?.stop()
     }
   }
 }