Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Dec 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion packages/playground/legacy/__tests__/legacy.spec.ts
Original file line number Diff line number Diff line change
@@ -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')
Expand Down Expand Up @@ -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)
})
}
22 changes: 5 additions & 17 deletions packages/plugin-legacy/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-legacy/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@
"systemjs": "^6.11.0"
},
"peerDependencies": {
"vite": "^2.0.0"
"vite": "^2.7.8"
}
}
2 changes: 1 addition & 1 deletion packages/vite/src/node/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
45 changes: 31 additions & 14 deletions packages/vite/src/node/plugins/terser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -41,7 +58,7 @@ export function terserPlugin(config: ResolvedConfig): Plugin {
},

closeBundle() {
worker.stop()
worker?.stop()
}
}
}