From 9b741afa8357ecb603ee25902ba0fedabbbdd3ed Mon Sep 17 00:00:00 2001 From: sunnylost Date: Sat, 14 Sep 2024 00:29:50 +0800 Subject: [PATCH 1/4] fix(minify): set the default value of esbuild's legalComments option to external --- packages/vite/src/node/plugins/css.ts | 3 +- packages/vite/src/node/plugins/esbuild.ts | 22 +++++++ playground/minify/__tests__/minify.spec.ts | 60 ++++++++++++++++--- playground/minify/__tests__/serve.ts | 39 ++++++++++++ .../minify/vite.legal-comments-eof.config.js | 11 ++++ .../vite.legal-comments-external.config.js | 11 ++++ .../vite.legal-comments-inline.config.js | 11 ++++ ....js => vite.legal-comments-none.config.js} | 3 + 8 files changed, 150 insertions(+), 10 deletions(-) create mode 100644 playground/minify/__tests__/serve.ts create mode 100644 playground/minify/vite.legal-comments-eof.config.js create mode 100644 playground/minify/vite.legal-comments-external.config.js create mode 100644 playground/minify/vite.legal-comments-inline.config.js rename playground/minify/{vite.config.js => vite.legal-comments-none.config.js} (77%) diff --git a/packages/vite/src/node/plugins/css.ts b/packages/vite/src/node/plugins/css.ts index 390804543ce9e3..48b4d1eb0475a0 100644 --- a/packages/vite/src/node/plugins/css.ts +++ b/packages/vite/src/node/plugins/css.ts @@ -1885,7 +1885,8 @@ function resolveMinifyCssEsbuildOptions( logLevel: options.logLevel, logLimit: options.logLimit, logOverride: options.logOverride, - legalComments: options.legalComments, + legalComments: + options.legalComments === 'linked' ? 'external' : options.legalComments, } if ( diff --git a/packages/vite/src/node/plugins/esbuild.ts b/packages/vite/src/node/plugins/esbuild.ts index c4d86432d5f320..4119bb03e5337f 100644 --- a/packages/vite/src/node/plugins/esbuild.ts +++ b/packages/vite/src/node/plugins/esbuild.ts @@ -53,6 +53,7 @@ export interface ESBuildOptions extends TransformOptions { * This option is not respected. Use `build.minify` instead. */ minify?: never + legalComments?: 'none' | 'inline' | 'eof' | 'linked' | 'external' } export type ESBuildTransformResult = Omit & { @@ -165,6 +166,7 @@ export async function transformWithEsbuild( } const resolvedOptions: TransformOptions = { + legalComments: 'external', sourcemap: true, // ensure source file name contains full query sourcefile: filename, @@ -173,6 +175,10 @@ export async function transformWithEsbuild( tsconfigRaw, } + if (resolvedOptions.legalComments === 'linked') { + resolvedOptions.legalComments = 'external' + } + // Some projects in the ecosystem are calling this function with an ESBuildOptions // object and esbuild throws an error for extra fields // @ts-expect-error include exists in ESBuildOptions @@ -303,6 +309,8 @@ const rollupToEsbuildFormatMap: Record< } export const buildEsbuildPlugin = (config: ResolvedConfig): Plugin => { + const collectLegalComments: string[] = [] + return { name: 'vite:esbuild-transpile', async renderChunk(code, chunk, opts) { @@ -319,6 +327,10 @@ export const buildEsbuildPlugin = (config: ResolvedConfig): Plugin => { const res = await transformWithEsbuild(code, chunk.fileName, options) + if (res.legalComments) { + collectLegalComments.push(res.legalComments) + } + if (config.build.lib) { // #7188, esbuild adds helpers out of the UMD and IIFE wrappers, and the // names are minified potentially causing collision with other globals. @@ -348,6 +360,16 @@ export const buildEsbuildPlugin = (config: ResolvedConfig): Plugin => { return res }, + + generateBundle(options) { + if (collectLegalComments.length && options.dir) { + this.emitFile({ + fileName: '.LEGAL.txt', + type: 'asset', + source: collectLegalComments.join('\n'), + }) + } + }, } } diff --git a/playground/minify/__tests__/minify.spec.ts b/playground/minify/__tests__/minify.spec.ts index 7b672d21134257..95aa3bafebd227 100644 --- a/playground/minify/__tests__/minify.spec.ts +++ b/playground/minify/__tests__/minify.spec.ts @@ -1,21 +1,63 @@ import fs from 'node:fs' import path from 'node:path' -import { expect, test } from 'vitest' +import { describe, expect, test } from 'vitest' import { isBuild, readFile, testDir } from '~utils' -test.runIf(isBuild)('no minifySyntax', () => { - const assetsDir = path.resolve(testDir, 'dist/assets') +function getJsAndCssContent(assetsDir: string) { const files = fs.readdirSync(assetsDir) const jsFile = files.find((f) => f.endsWith('.js')) - const jsContent = readFile(path.resolve(assetsDir, jsFile)) + const jsContent = readFile(path.resolve(assetsDir, jsFile)).trim() const cssFile = files.find((f) => f.endsWith('.css')) - const cssContent = readFile(path.resolve(assetsDir, cssFile)) + const cssContent = readFile(path.resolve(assetsDir, cssFile)).trim() - expect(jsContent).toContain('{console.log("hello world")}') - expect(jsContent).not.toContain('/*! explicit comment */') + return { jsContent, cssContent } +} - expect(cssContent).toContain('color:#ff0000') - expect(cssContent).not.toContain('/*! explicit comment */') +function getLegalFileContent(assetsDir: string) { + return readFile(path.resolve(assetsDir, '.LEGAL.txt')) +} + +describe.runIf(isBuild)('minify', () => { + test('Do not preserve any legal comments', () => { + const { jsContent, cssContent } = getJsAndCssContent( + path.resolve(testDir, 'dist/none/assets'), + ) + + expect(jsContent).toContain('{console.log("hello world")}') + expect(jsContent).not.toContain('/*! explicit comment */') + + expect(cssContent).toContain('color:#ff0000') + expect(cssContent).not.toContain('/*! explicit comment */') + }) + + test('Preserve legal comments', () => { + const { jsContent, cssContent } = getJsAndCssContent( + path.resolve(testDir, 'dist/inline/assets'), + ) + + expect(jsContent).toContain('/*! explicit comment */') + expect(cssContent).toContain('/*! explicit comment */') + }) + + test('Move all legal comments to the end of the file.', () => { + const { jsContent, cssContent } = getJsAndCssContent( + path.resolve(testDir, 'dist/eof/assets'), + ) + expect(jsContent.endsWith('/*! explicit comment */')).toBeTruthy() + expect(cssContent.endsWith('/*! explicit comment */')).toBeTruthy() + }) + + test('Move all legal comments to a .LEGAL.txt file but to not link to them.', () => { + const { jsContent, cssContent } = getJsAndCssContent( + path.resolve(testDir, 'dist/external/assets'), + ) + const legaContent = getLegalFileContent( + path.resolve(testDir, 'dist/external'), + ) + expect(jsContent).not.toContain('/*! explicit comment */') + expect(cssContent).not.toContain('/*! explicit comment */') + expect(legaContent).toContain('/*! explicit comment */') + }) }) diff --git a/playground/minify/__tests__/serve.ts b/playground/minify/__tests__/serve.ts new file mode 100644 index 00000000000000..048e9a4fcd8aac --- /dev/null +++ b/playground/minify/__tests__/serve.ts @@ -0,0 +1,39 @@ +// this is automatically detected by playground/vitestSetup.ts and will replace +// the default e2e test serve behavior + +import path from 'node:path' +import { ports, rootDir } from '~utils' + +export const port = ports.lib + +export async function serve() { + const { build } = await import('vite') + await build({ + root: rootDir, + configFile: path.resolve(__dirname, '../vite.legal-comments-eof.config.js'), + }) + + await build({ + root: rootDir, + configFile: path.resolve( + __dirname, + '../vite.legal-comments-external.config.js', + ), + }) + + await build({ + root: rootDir, + configFile: path.resolve( + __dirname, + '../vite.legal-comments-inline.config.js', + ), + }) + + await build({ + root: rootDir, + configFile: path.resolve( + __dirname, + '../vite.legal-comments-none.config.js', + ), + }) +} diff --git a/playground/minify/vite.legal-comments-eof.config.js b/playground/minify/vite.legal-comments-eof.config.js new file mode 100644 index 00000000000000..f01c5862228336 --- /dev/null +++ b/playground/minify/vite.legal-comments-eof.config.js @@ -0,0 +1,11 @@ +import { defineConfig } from 'vite' + +export default defineConfig({ + esbuild: { + legalComments: 'eof', + minifySyntax: false, + }, + build: { + outDir: 'dist/eof', + }, +}) diff --git a/playground/minify/vite.legal-comments-external.config.js b/playground/minify/vite.legal-comments-external.config.js new file mode 100644 index 00000000000000..f2c0830de16da1 --- /dev/null +++ b/playground/minify/vite.legal-comments-external.config.js @@ -0,0 +1,11 @@ +import { defineConfig } from 'vite' + +export default defineConfig({ + esbuild: { + legalComments: 'external', + minifySyntax: false, + }, + build: { + outDir: 'dist/external', + }, +}) diff --git a/playground/minify/vite.legal-comments-inline.config.js b/playground/minify/vite.legal-comments-inline.config.js new file mode 100644 index 00000000000000..8849a1d0b7f42d --- /dev/null +++ b/playground/minify/vite.legal-comments-inline.config.js @@ -0,0 +1,11 @@ +import { defineConfig } from 'vite' + +export default defineConfig({ + esbuild: { + legalComments: 'inline', + minifySyntax: false, + }, + build: { + outDir: 'dist/inline', + }, +}) diff --git a/playground/minify/vite.config.js b/playground/minify/vite.legal-comments-none.config.js similarity index 77% rename from playground/minify/vite.config.js rename to playground/minify/vite.legal-comments-none.config.js index 018cc196e60707..bb71cebaa0a232 100644 --- a/playground/minify/vite.config.js +++ b/playground/minify/vite.legal-comments-none.config.js @@ -5,4 +5,7 @@ export default defineConfig({ legalComments: 'none', minifySyntax: false, }, + build: { + outDir: 'dist/none', + }, }) From 874277fb4a371c3aa573070e60e37c99801d871d Mon Sep 17 00:00:00 2001 From: sunnylost Date: Sun, 15 Sep 2024 17:08:40 +0800 Subject: [PATCH 2/4] fix(minify): improve legalComments:linked option --- packages/vite/src/node/plugins/esbuild.ts | 6 +++++- playground/minify/__tests__/minify.spec.ts | 19 +++++++++++++++++-- playground/minify/__tests__/serve.ts | 8 ++++++++ .../vite.legal-comments-linked.config.js | 11 +++++++++++ 4 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 playground/minify/vite.legal-comments-linked.config.js diff --git a/packages/vite/src/node/plugins/esbuild.ts b/packages/vite/src/node/plugins/esbuild.ts index 4119bb03e5337f..bc3f3b95acc5a5 100644 --- a/packages/vite/src/node/plugins/esbuild.ts +++ b/packages/vite/src/node/plugins/esbuild.ts @@ -329,6 +329,10 @@ export const buildEsbuildPlugin = (config: ResolvedConfig): Plugin => { if (res.legalComments) { collectLegalComments.push(res.legalComments) + + if (config.esbuild && config.esbuild.legalComments === 'linked') { + res.code += '\n/*! For license information please see LEGAL.txt */' + } } if (config.build.lib) { @@ -364,7 +368,7 @@ export const buildEsbuildPlugin = (config: ResolvedConfig): Plugin => { generateBundle(options) { if (collectLegalComments.length && options.dir) { this.emitFile({ - fileName: '.LEGAL.txt', + fileName: 'LEGAL.txt', type: 'asset', source: collectLegalComments.join('\n'), }) diff --git a/playground/minify/__tests__/minify.spec.ts b/playground/minify/__tests__/minify.spec.ts index 95aa3bafebd227..c42fc51c05857b 100644 --- a/playground/minify/__tests__/minify.spec.ts +++ b/playground/minify/__tests__/minify.spec.ts @@ -16,7 +16,7 @@ function getJsAndCssContent(assetsDir: string) { } function getLegalFileContent(assetsDir: string) { - return readFile(path.resolve(assetsDir, '.LEGAL.txt')) + return readFile(path.resolve(assetsDir, 'LEGAL.txt')) } describe.runIf(isBuild)('minify', () => { @@ -49,7 +49,7 @@ describe.runIf(isBuild)('minify', () => { expect(cssContent.endsWith('/*! explicit comment */')).toBeTruthy() }) - test('Move all legal comments to a .LEGAL.txt file but to not link to them.', () => { + test('Move all legal comments to a LEGAL.txt file but to not link to them.', () => { const { jsContent, cssContent } = getJsAndCssContent( path.resolve(testDir, 'dist/external/assets'), ) @@ -60,4 +60,19 @@ describe.runIf(isBuild)('minify', () => { expect(cssContent).not.toContain('/*! explicit comment */') expect(legaContent).toContain('/*! explicit comment */') }) + + test('Move all legal comments to a LEGAL.txt file and link to them with a comment.', () => { + const { jsContent, cssContent } = getJsAndCssContent( + path.resolve(testDir, 'dist/linked/assets'), + ) + const legaContent = getLegalFileContent( + path.resolve(testDir, 'dist/linked'), + ) + expect(jsContent).not.toContain('/*! explicit comment */') + expect(jsContent).toContain( + '/*! For license information please see LEGAL.txt */', + ) + expect(cssContent).not.toContain('/*! explicit comment */') + expect(legaContent).toContain('/*! explicit comment */') + }) }) diff --git a/playground/minify/__tests__/serve.ts b/playground/minify/__tests__/serve.ts index 048e9a4fcd8aac..bc60c4d97a16f3 100644 --- a/playground/minify/__tests__/serve.ts +++ b/playground/minify/__tests__/serve.ts @@ -36,4 +36,12 @@ export async function serve() { '../vite.legal-comments-none.config.js', ), }) + + await build({ + root: rootDir, + configFile: path.resolve( + __dirname, + '../vite.legal-comments-linked.config.js', + ), + }) } diff --git a/playground/minify/vite.legal-comments-linked.config.js b/playground/minify/vite.legal-comments-linked.config.js new file mode 100644 index 00000000000000..ac1b2ad14bc7da --- /dev/null +++ b/playground/minify/vite.legal-comments-linked.config.js @@ -0,0 +1,11 @@ +import { defineConfig } from 'vite' + +export default defineConfig({ + esbuild: { + legalComments: 'linked', + minifySyntax: false, + }, + build: { + outDir: 'dist/linked', + }, +}) From 3b943bdd69abe7a93fcc05cc34d07ee9f8ae1ef8 Mon Sep 17 00:00:00 2001 From: sunnylost Date: Tue, 17 Sep 2024 15:31:02 +0800 Subject: [PATCH 3/4] fix(minify): update output filename, update legalComments' default value. --- packages/vite/src/node/plugins/esbuild.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/vite/src/node/plugins/esbuild.ts b/packages/vite/src/node/plugins/esbuild.ts index bc3f3b95acc5a5..91ea9bbbf7532a 100644 --- a/packages/vite/src/node/plugins/esbuild.ts +++ b/packages/vite/src/node/plugins/esbuild.ts @@ -166,7 +166,7 @@ export async function transformWithEsbuild( } const resolvedOptions: TransformOptions = { - legalComments: 'external', + legalComments: 'linked', sourcemap: true, // ensure source file name contains full query sourcefile: filename, @@ -368,7 +368,7 @@ export const buildEsbuildPlugin = (config: ResolvedConfig): Plugin => { generateBundle(options) { if (collectLegalComments.length && options.dir) { this.emitFile({ - fileName: 'LEGAL.txt', + name: 'LEGAL.txt', type: 'asset', source: collectLegalComments.join('\n'), }) From 33e81dc21a3f1df8b336cdf4232e9a0cc784371f Mon Sep 17 00:00:00 2001 From: sunnylost Date: Tue, 17 Sep 2024 16:14:22 +0800 Subject: [PATCH 4/4] fix(minify): ensure the css files contains legal comments link. --- packages/vite/src/node/plugins/esbuild.ts | 28 +++++++--- playground/minify/__tests__/minify.spec.ts | 62 ++++++++++++---------- 2 files changed, 55 insertions(+), 35 deletions(-) diff --git a/packages/vite/src/node/plugins/esbuild.ts b/packages/vite/src/node/plugins/esbuild.ts index 91ea9bbbf7532a..8877179e285e08 100644 --- a/packages/vite/src/node/plugins/esbuild.ts +++ b/packages/vite/src/node/plugins/esbuild.ts @@ -329,10 +329,6 @@ export const buildEsbuildPlugin = (config: ResolvedConfig): Plugin => { if (res.legalComments) { collectLegalComments.push(res.legalComments) - - if (config.esbuild && config.esbuild.legalComments === 'linked') { - res.code += '\n/*! For license information please see LEGAL.txt */' - } } if (config.build.lib) { @@ -365,13 +361,33 @@ export const buildEsbuildPlugin = (config: ResolvedConfig): Plugin => { return res }, - generateBundle(options) { + generateBundle(options, bundle) { if (collectLegalComments.length && options.dir) { - this.emitFile({ + const referenceId = this.emitFile({ name: 'LEGAL.txt', type: 'asset', source: collectLegalComments.join('\n'), }) + + if (config.esbuild && config.esbuild.legalComments === 'linked') { + const legalCommentsFileName = this.getFileName(referenceId) + const linkedComments = `/*! For license information please see ${legalCommentsFileName} */` + + for (const file in bundle) { + const chunk = bundle[file] + + if ( + chunk.fileName.endsWith('.css') || + chunk.fileName.endsWith('.js') + ) { + if (chunk.type === 'asset') { + chunk.source += linkedComments + } else if (chunk.type === 'chunk') { + chunk.code += linkedComments + } + } + } + } } }, } diff --git a/playground/minify/__tests__/minify.spec.ts b/playground/minify/__tests__/minify.spec.ts index c42fc51c05857b..8e85c5a96004a2 100644 --- a/playground/minify/__tests__/minify.spec.ts +++ b/playground/minify/__tests__/minify.spec.ts @@ -16,9 +16,17 @@ function getJsAndCssContent(assetsDir: string) { } function getLegalFileContent(assetsDir: string) { - return readFile(path.resolve(assetsDir, 'LEGAL.txt')) + const files = fs.readdirSync(assetsDir) + + for (const fileName of files) { + if (fileName.startsWith('LEGAL')) { + return readFile(path.join(assetsDir, fileName)) + } + } } +const legalComments = '/*! explicit comment */' + describe.runIf(isBuild)('minify', () => { test('Do not preserve any legal comments', () => { const { jsContent, cssContent } = getJsAndCssContent( @@ -26,10 +34,9 @@ describe.runIf(isBuild)('minify', () => { ) expect(jsContent).toContain('{console.log("hello world")}') - expect(jsContent).not.toContain('/*! explicit comment */') - + expect(jsContent).not.toContain(legalComments) expect(cssContent).toContain('color:#ff0000') - expect(cssContent).not.toContain('/*! explicit comment */') + expect(cssContent).not.toContain(legalComments) }) test('Preserve legal comments', () => { @@ -37,42 +44,39 @@ describe.runIf(isBuild)('minify', () => { path.resolve(testDir, 'dist/inline/assets'), ) - expect(jsContent).toContain('/*! explicit comment */') - expect(cssContent).toContain('/*! explicit comment */') + expect(jsContent).toContain(legalComments) + expect(cssContent).toContain(legalComments) }) test('Move all legal comments to the end of the file.', () => { const { jsContent, cssContent } = getJsAndCssContent( path.resolve(testDir, 'dist/eof/assets'), ) - expect(jsContent.endsWith('/*! explicit comment */')).toBeTruthy() - expect(cssContent.endsWith('/*! explicit comment */')).toBeTruthy() + + expect(jsContent.endsWith(legalComments)).toBeTruthy() + expect(cssContent.endsWith(legalComments)).toBeTruthy() }) test('Move all legal comments to a LEGAL.txt file but to not link to them.', () => { - const { jsContent, cssContent } = getJsAndCssContent( - path.resolve(testDir, 'dist/external/assets'), - ) - const legaContent = getLegalFileContent( - path.resolve(testDir, 'dist/external'), - ) - expect(jsContent).not.toContain('/*! explicit comment */') - expect(cssContent).not.toContain('/*! explicit comment */') - expect(legaContent).toContain('/*! explicit comment */') + const assetsPath = path.resolve(testDir, 'dist/external/assets') + const { jsContent, cssContent } = getJsAndCssContent(assetsPath) + const legaContent = getLegalFileContent(assetsPath) + + expect(jsContent).not.toContain(legalComments) + expect(cssContent).not.toContain(legalComments) + expect(legaContent).toContain(legalComments) }) test('Move all legal comments to a LEGAL.txt file and link to them with a comment.', () => { - const { jsContent, cssContent } = getJsAndCssContent( - path.resolve(testDir, 'dist/linked/assets'), - ) - const legaContent = getLegalFileContent( - path.resolve(testDir, 'dist/linked'), - ) - expect(jsContent).not.toContain('/*! explicit comment */') - expect(jsContent).toContain( - '/*! For license information please see LEGAL.txt */', - ) - expect(cssContent).not.toContain('/*! explicit comment */') - expect(legaContent).toContain('/*! explicit comment */') + const linkedCommentsPre = 'For license information please see' + const assetsPath = path.resolve(testDir, 'dist/linked/assets') + const { jsContent, cssContent } = getJsAndCssContent(assetsPath) + const legaContent = getLegalFileContent(assetsPath) + + expect(jsContent).not.toContain(legalComments) + expect(jsContent.includes(linkedCommentsPre)).toBeTruthy() + expect(cssContent).not.toContain(legalComments) + expect(cssContent.includes(linkedCommentsPre)).toBeTruthy() + expect(legaContent).toContain(legalComments) }) })