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

feat: respect esbuild minify config for css #8811

Merged
merged 3 commits into from Jun 27, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/config/shared-options.md
Expand Up @@ -280,7 +280,7 @@ export default defineConfig({
})
```

When [`build.minify`](./build-options.md#build-minify) is `true`, you can configure to only minify [certain aspects](https://esbuild.github.io/api/#minify) of the code by setting either of `esbuild.minifyIdentifiers`, `esbuild.minifySyntax`, and `esbuild.minifyWhitespace` to `true`. Note the `esbuild.minify` option can't be used to override `build.minify`.
When [`build.minify`](./build-options.md#build-minify) is `true`, all minify optimizations are applied by default. To disable [certain aspects](https://esbuild.github.io/api/#minify) of it, set any of `esbuild.minifyIdentifiers`, `esbuild.minifySyntax`, or `esbuild.minifyWhitespace` options to `false`. Note the `esbuild.minify` option can't be used to override `build.minify`.

Set to `false` to disable esbuild transforms.

Expand Down
29 changes: 28 additions & 1 deletion packages/vite/src/node/__tests__/plugins/esbuild.spec.ts
Expand Up @@ -41,6 +41,31 @@ describe('resolveEsbuildTranspileOptions', () => {
expect(options).toEqual(null)
})

test('resolve specific minify options', () => {
const options = resolveEsbuildTranspileOptions(
defineResolvedConfig({
build: {
minify: 'esbuild'
},
esbuild: {
keepNames: true,
minifyIdentifiers: false
}
}),
'es'
)
expect(options).toEqual({
target: undefined,
format: 'esm',
keepNames: true,
minify: false,
minifyIdentifiers: false,
minifySyntax: true,
minifyWhitespace: true,
treeShaking: true
})
})

test('resolve no minify', () => {
const options = resolveEsbuildTranspileOptions(
defineResolvedConfig({
Expand Down Expand Up @@ -140,6 +165,7 @@ describe('resolveEsbuildTranspileOptions', () => {
keepNames: true,
minify: false,
minifyIdentifiers: true,
minifySyntax: true,
minifyWhitespace: false,
treeShaking: true
})
Expand All @@ -157,7 +183,7 @@ describe('resolveEsbuildTranspileOptions', () => {
esbuild: {
keepNames: true,
minifyIdentifiers: true,
minifyWhitespace: true,
minifySyntax: false,
treeShaking: true
}
}),
Expand All @@ -169,6 +195,7 @@ describe('resolveEsbuildTranspileOptions', () => {
keepNames: true,
minify: false,
minifyIdentifiers: true,
minifySyntax: false,
minifyWhitespace: true,
treeShaking: true
})
Expand Down
24 changes: 22 additions & 2 deletions packages/vite/src/node/plugins/css.ts
Expand Up @@ -19,6 +19,7 @@ import type Sass from 'sass'
import type Stylus from 'stylus'
import type Less from 'less'
import type { Alias } from 'types/alias'
import type { TransformOptions } from 'esbuild'
import { formatMessages, transform } from 'esbuild'
import type { RawSourceMap } from '@ampproject/remapping'
import { getCodeWithSourcemap, injectSourcesContent } from '../server/sourcemap'
Expand Down Expand Up @@ -55,6 +56,7 @@ import {
publicFileToBuiltUrl,
resolveAssetFileNames
} from './asset'
import type { ESBuildOptions } from './esbuild'

// const debug = createDebugger('vite:css')

Expand Down Expand Up @@ -1211,8 +1213,8 @@ async function minifyCSS(css: string, config: ResolvedConfig) {
try {
const { code, warnings } = await transform(css, {
loader: 'css',
minify: true,
target: config.build.cssTarget || undefined
target: config.build.cssTarget || undefined,
...resolveEsbuildMinifyOptions(config.esbuild || {})
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be nicer if we could pre-compute this somewhere, but I can't find a good spot to put without making the flow a bit awkward. Perhaps it's fine with V8

})
if (warnings.length) {
const msgs = await formatMessages(warnings, { kind: 'warning' })
Expand All @@ -1231,6 +1233,24 @@ async function minifyCSS(css: string, config: ResolvedConfig) {
}
}

function resolveEsbuildMinifyOptions(
options: ESBuildOptions
): TransformOptions {
if (
options.minifyIdentifiers != null ||
options.minifySyntax != null ||
options.minifyWhitespace != null
) {
return {
minifyIdentifiers: options.minifyIdentifiers ?? true,
minifySyntax: options.minifySyntax ?? true,
minifyWhitespace: options.minifyWhitespace ?? true
}
} else {
return { minify: true }
}
}

export async function hoistAtRules(css: string): Promise<string> {
const s = new MagicString(css)
const cleanCss = emptyCssComments(css)
Expand Down
11 changes: 8 additions & 3 deletions packages/vite/src/node/plugins/esbuild.ts
Expand Up @@ -318,22 +318,27 @@ export function resolveEsbuildTranspileOptions(

// If user enable fine-grain minify options, minify with their options instead
if (
options.minifyIdentifiers ||
options.minifySyntax ||
options.minifyWhitespace
options.minifyIdentifiers != null ||
options.minifySyntax != null ||
options.minifyWhitespace != null
) {
if (isEsLibBuild) {
// Disable minify whitespace as it breaks tree-shaking
return {
...options,
minify: false,
minifyIdentifiers: options.minifyIdentifiers ?? true,
minifySyntax: options.minifySyntax ?? true,
minifyWhitespace: false,
treeShaking: true
}
} else {
return {
...options,
minify: false,
minifyIdentifiers: options.minifyIdentifiers ?? true,
minifySyntax: options.minifySyntax ?? true,
minifyWhitespace: options.minifyWhitespace ?? true,
treeShaking: true
}
}
Expand Down
18 changes: 18 additions & 0 deletions playground/minify/__tests__/minify.spec.ts
@@ -0,0 +1,18 @@
import fs from 'node:fs'
import path from 'node:path'
import { expect, test } from 'vitest'
import { isBuild, readFile, testDir } from '~utils'

test.runIf(isBuild)('no minifySyntax', () => {
const assetsDir = path.resolve(testDir, 'dist/assets')
const files = fs.readdirSync(assetsDir)

const jsFile = files.find((f) => f.endsWith('.js'))
const jsContent = readFile(path.resolve(assetsDir, jsFile))

const cssFile = files.find((f) => f.endsWith('.css'))
const cssContent = readFile(path.resolve(assetsDir, cssFile))

expect(jsContent).toContain('{console.log("hello world")}')
expect(cssContent).toContain('color:#ff0000')
})
3 changes: 3 additions & 0 deletions playground/minify/index.html
@@ -0,0 +1,3 @@
<h1>Minify</h1>

<script type="module" src="./main.js"></script>
5 changes: 5 additions & 0 deletions playground/minify/main.js
@@ -0,0 +1,5 @@
import './test.css'

if (window) {
console.log('hello world')
}
11 changes: 11 additions & 0 deletions playground/minify/package.json
@@ -0,0 +1,11 @@
{
"name": "test-minify",
"private": true,
"version": "0.0.0",
"scripts": {
"dev": "vite",
"build": "vite build",
"debug": "node --inspect-brk ../../packages/vite/bin/vite",
"preview": "vite preview"
}
}
4 changes: 4 additions & 0 deletions playground/minify/test.css
@@ -0,0 +1,4 @@
h1 {
/* do not minify as red text */
color: #ff0000;
}
7 changes: 7 additions & 0 deletions playground/minify/vite.config.js
@@ -0,0 +1,7 @@
import { defineConfig } from 'vite'

export default defineConfig({
esbuild: {
minifySyntax: false
}
})