vite.config.js esbuild options are ignored, e.g. disabling minification or one of its aspects

[kskalski]

Describe the bug

I'm trying to control how esbuild is performing minification due to some recent features that I would like to disable (evanw/esbuild#1755), however it seems that options specified in vite.config.js by esbuild section are ignored, e.g. setting minify: false there doesn't have any effect:

import { defineConfig } from 'vite'
import vue from '@vitejs/plugin-vue'

// https://vitejs.dev/config/
export default defineConfig({
  esbuild: {
    minify: false,
    minifySyntax: false
  },
  plugins: [vue()]
})

Reproduction

This repo shows the problem
https://github.com/kskalski/snippets/tree/master/esbuild-minify-test

to preproduce:

1. npm i
2. npm run build
3. grep "\(foo\|bar\)" dist/assets/index.*.css shows the rule .foo,.bar{color:red} while original App.vue has them as separate rules

Note that it is possible to completely disable minification by option

{
  build: {
    minify: false
  }
}

however I would like to have more fine grained control (affect specific esbuild options instead of completely disabling the pipeline). Also related is this issue #5619, which asks for splitting options between JS and CSS minification, here however I'm observing that esbuild config section is not correctly applied (though it is validated for correct property names).

System Info

System:
    OS: Linux 4.4 Ubuntu 21.04 (Hirsute Hippo)
    CPU: (8) x64 Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
    Memory: 1.39 GB / 7.89 GB
    Container: Yes
    Shell: 5.1.4 - /bin/bash
  Binaries:
    Node: 12.21.0 - /usr/bin/node
    npm: 7.5.2 - /usr/bin/npm
  npmPackages:
    @vitejs/plugin-vue: ^1.10.2 => 1.10.2
    vite: ^2.7.0 => 2.7.1

Used Package Manager

npm

Logs

> vite build --debug

  vite:config bundled config file loaded in 177.49ms +0ms
  vite:config using resolved config: {
  vite:config   esbuild: { minify: false, minifySyntax: false },
...

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to https://github.com/vuejs/vue-next instead.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[rosostolato]

Any news on it?

[kyr0]

@kskalski @rosostolato I'd suggest to use the right property name :) It is build, not esbuild:

export default defineConfig({
  build: {
    minify: false,
  },
  ...
})

And then it magically works ;)

[kskalski]

@kyr0 as mentioned in this issue's description:

Note that it is possible to completely disable minification by option

{
  build: {
    minify: false
  }
}

however I would like to have more fine grained control (affect specific esbuild options instead of completely disabling the pipeline).

As mentioned in documentation (https://vitejs.dev/config/#esbuild) vite config has esbuild section, but it's ignored.

[bodograumann]

The relevant code is here: https://github.com/vitejs/vite/blob/main/packages/vite/src/node/plugins/esbuild.ts#L251

So minify can only be set in build, but most other esbuild options should be usable in the esbuild object. I personally tried setting keepNames and it worked fine.

[kskalski]

I care most about minifySyntax, but setting it either in esbuild section or build section doesn't have proper effect. See repro in https://github.com/kskalski/snippets/blob/master/esbuild-minify-test/README.md

[brandonkramer]

Yup, wanted to set minifyIdentifiers to false but it's getting ignored in both esbuild, optimizeDeps.esbuildOptions and build

[kskalski]

I bumped the repro to vite 3.0.0-beta.3, unfortunately the esbuild options are still ignored
https://github.com/kskalski/snippets/tree/master/esbuild-minify-test

[kskalski]

I guess there is still an override that forces minifySyntax: true here:

  // Else apply default minify options
  if (isEsLibBuild) {
    // Minify all except whitespace as it breaks tree-shaking
    return {
      ...options,
      minify: false,
      minifyIdentifiers: true,
      minifySyntax: true,

(https://github.com/vitejs/vite/pull/8754/files#)

what is "es lib build" and can I choose in vite.config.js whether I'm using it or not?

[bluwy]

The esbuild options are not ignored. The options for minify and minifySyntax are false by default (following esbuild itself), so the config in your repro doesn't do anything. If you want to minify all except minifySyntax, you can specify minifyWhitespace: true and minifyIdentifiers: true. Docs.

[sapphi-red]

Since we are defaulting minify to true, it might be intuitive to make minifySyntax and others as true for default.
Or maybe require users to set all options, if any of them are set.
My preference is the latter one because we have a opposite default to esbuild.

[kskalski]

I believe my issue is related to css files only and the (different than for .js files) options that are passed to esbuild. E.g. in my repro I added some debug to print which options are passed to esbuild and it differs for files other than .js output:

esbuild opts {
  loader: 'css',
  minify: true,
  target: [ 'es2020', 'edge88', 'firefox78', 'chrome87', 'safari13' ]
}
esbuild opts {
  sourcemap: true,
  sourcefile: 'assets/index.ef6f7eb0.js',
  minify: false,
  minifySyntax: false,
  minifyIdentifiers: true,

Is it possible that there is some css loader pipeline, which just passes minify: true without following the vite.config.js options?
In any case, @bluwy if you claim that my repro should work with the options applied (or without them if as you say minify syntax is disabled by default), then could you run it and see on your own that it still doesn't?

[kskalski]

Basically the problem is with this line

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

Line 1208 in c1667bb

minify: true,

async function minifyCSS(css: string, config: ResolvedConfig) {
  try {
    const { code, warnings } = await transform(css, {
      loader: 'css',
      minify: true,

it doesn't follow the user config, tweaking it solves my issue.

[bluwy]

Ah good fine. Yeah I forgot to update the CSS part.

Since we are defaulting minify to true, it might be intuitive to make minifySyntax and others as true for default.
Or maybe require users to set all options, if any of them are set.
My preference is the latter one because we have a opposite default to esbuild.

I've thought about the different options while making the PR and landed with the current one as it makes reasoning the final options a bit easier. I'm open if anyone has thoughts or prefer an alternative resolve logic though and we can still change it before stable. Re the latter option, I'm not sure having the user to set all the options by default would be good for DX.

[kskalski]

Since we are defaulting minify to true, it might be intuitive to make minifySyntax and others as true for default. Or maybe require users to set all options, if any of them are set. My preference is the latter one because we have a opposite default to esbuild.

@sapphi-red the way esbuild interprets options in https://github.com/evanw/esbuild/blob/0d4fae1f97b2013b78a293f1f19607df062a52f1/lib/shared/common.ts#L159-L162
in fact requires you to set minify: false for any other option to take effect, so this might be one source of confusion.
Another part of confusion might come from the way this condition

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

Lines 319 to 324 in 8b77695

	// If user enable fine-grain minify options, minify with their options instead
	if (
	options.minifyIdentifiers ||
	options.minifySyntax ||
	options.minifyWhitespace
	) {

is only satisfied for true values in one of those fields, setting explicitly false on them will be ignored. I would definitely modify this check to something like minifyIdentifiers != null

[bluwy]

Currently writing tests for it and I can see how setting minifyWhitespace: false feels more natural than setting to true. I'll push a fix for the CSS and change to use != null, and resolving the options to something like:

// when one of `minify*` options are set (checked by != null)
 if ( 
   options.minifyIdentifiers != null || 
   options.minifySyntax != null || 
   options.minifyWhitespace != null
) { 
return {
  minifyIdentifiers: options.minifyIdentifiers != null ? options.minifyIdentifiers : true,
 minifySyntax: options.minifySyntax != null ? options.minifySyntax : true,
 minifyWhitespace: options.minifyWhitespace != null ? options.minifyWhitespace : true,
}
}

when minifying. Curious to here thoughts on this so we don't have to change again 😅

[sapphi-red]

That sounds good to me. 👍 (A nit pick, using ?? would shorten the code.)