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

Automatically pick up .terserrc #389

Open
fregante opened this issue Apr 9, 2021 · 15 comments
Open

Automatically pick up .terserrc #389

fregante opened this issue Apr 9, 2021 · 15 comments

Comments

@fregante
Copy link

fregante commented Apr 9, 2021

  • webpack Version: 5.31.0
  • terser-webpack-plugin Version: 5.1.1

Feature Proposal

Configuration could be automatically picked up by looking for .terserrc in the same directory as webpack.config.js like Parcel does:
https://github.com/parcel-bundler/parcel/blob/39ff8330d68c6c0e01d9e8471dd94ce1eb2e62ec/packages/optimizers/terser/src/TerserOptimizer.js#L23

Feature Use Case

Parcel v2 lets me customize the minification step of Tercer by using a configuration file. It'd be great if webpack did this natively as well so that I don't have to install, require, and call new TerserPlugin() in webpack.config.js

.terserc also seen on the standard-things/esm repo, which loads it manually.

@alexander-akait
Copy link
Member

We avoid it because it is not official and reduce perf, better load once options and don't touch fs once again

@fregante
Copy link
Author

fregante commented Apr 9, 2021

it is not official

Isn't this a webpack repo? Adding it here would make it official for webpack users like webpack made webpack.config.js official.

Direct Terser support (which is probably what you mean by "official") could be suggested there, but probably only for its CLI tool anyway, not the API used by this plugin.

better load once options and don't touch fs once again

In the context of webpack I don't think a single 1KB read affects the performance in any significant way whatsoever.

@alexander-akait
Copy link
Member

Direct Terser support (which is probably what you mean by "official") could be suggested there, but probably only for its CLI tool anyway, not the API used by this plugin.

Yes, I mean terser is not supported configurations, first I would like to discuss this with @fabiosantoscode. Why? Because parcel invents formats, for example postcss configurations is not compatibility with postcss-cli and postcss other utils and I want to avoid it here. If we approve how configuration should look (i.e. options and formats) I will implement this.

In the context of webpack I don't think a single 1KB read affects the performance in any significant way whatsoever.

const terserOptions = require('.terserc') is less and no need extra readings and will be cached by Node.js, just my opinion

@fregante
Copy link
Author

fregante commented Apr 9, 2021

const terserOptions = require('.terserc') is less and no need extra readings and will be cached by Node.js, just my opinion

Maybe I should clarify that the intent of this issue is not necessarily to have external configuration, which isn't really useful since webpack already has its own configuration file, but to be able to customize Terser’s options without having to

install, require, and call new TerserPlugin() in webpack.config.js

require('.terserc') doesn't solve that

@alexander-akait
Copy link
Member

Maybe I should clarify that the intent of this issue is not necessarily to have external configuration, which isn't really useful since webpack already has its own configuration file, but to be able to customize Terser’s options without having to

I understand this, just want agree format of configuration and supported extensions.

@fabiosantoscode
Copy link
Contributor

Hello!

Terser already has a config file loading function, --config-file, but only on the CLI.

The configuration file has basically the same format as the first argument to minify(), with some differences pertaining to the fact that strings are used to represent regexes.

I'm delighted with this .terserrc thing, informal standards are awesome!

I'm happy to allow users to specify the path to it on the API like is possible with the CLI (it might be a thing already, I'm not sure). Then webpack can find this file and pass its path over.

@fregante
Copy link
Author

fregante commented Apr 9, 2021

informal standards are awesome!

🙌

specify the path to it on the API

Again this defeats the purpose of having a file. If it's not picked up automatically this isn't useful.

Currently

	optimization: {
		// Keeps it somewhat readable for AMO reviewers
		minimizer: [
			new TerserPlugin({
				terserOptions: {
					mangle: false,
					output: {
						beautify: true,
						indent_level: 2
					}
				}
			})
		]
	}

Currently also possible

	optimization: {
		// Keeps it somewhat readable for AMO reviewers
		minimizer: [
			new TerserPlugin({
				terserOptions: require('./.terserrc')
			})
		]
	}

What you're suggesting

	optimization: {
		// Keeps it somewhat readable for AMO reviewers
		minimizer: [
			new TerserPlugin({
				terserOptionsPath: './.terserrc'
			})
		]
	}

What I really want

@fabiosantoscode
Copy link
Contributor

Love your ideal example here 😃 I would like to have that too, but there are a few reasons I'd prefer that webpack and other tools do this (potentially by importing some @terser/something module from npm)

  1. It's not ideal to rely on process.cwd() to find these rc files.
  2. How the rc file is found, should be configurable per bundler or user, such that it's possible to use multiple files in a monorepo without Terser imposing project organisation on you.
  3. It's not clear how the rc file should override configuration from the minify() argument or the other way around. It depends on your own use case.

So I don't mean to push back here -- I think a standard rc file is a very good goal to strive towards. I just want to make sure it's done right. Something like this has big implications (I had quite a few problems with .babelrc before) and I dislike releasing major versions because that causes other problems too.

@fabiosantoscode
Copy link
Contributor

As a side note, I see this as something that could help debugging Terser itself. Lots of people come up to Terser and open issues about how something is crashing, often not knowing enough details to help me help them. A configuration file opens up the possibility of a more detailed bug report (think a "bug report" mode where comments indicate source map position and you can tell why the crash is happening and isolate a reproducible snippet more easily)

@fregante
Copy link
Author

fregante commented Apr 9, 2021

  • It's not ideal to rely on process.cwd() to find these rc files.

Can the plugin access webpack --config value? I'd assume both files to be in the same folder, which is the root by default. No cwd calls needed.

2. How the rc file is found, should be configurable

I think that's thinking too far ahead. Given that there are other ways to achieve the same, this would be an enhancement rather than a requirement. At most I'd expect a --no-babelrc-like flag

3. It's not clear how the rc file should override configuration

IMHO it is clear: the closer to the user, the high the priority:

  1. Read from ENV <-- low
  2. Read from config file
  3. Get API options
  4. Get CLI flags <-- overrides all

This is what TypeScript does for example:

  1. Reads tsconfig.json
  2. CLI flags override any options

Also Babel has extensive documentation on the matter, showing this also: https://babeljs.io/docs/en/configuration

  • babel.config.json < .babelrc < programmatic options from @babel/cli

Rollup does the same: https://rollupjs.org/guide/en/#command-line-flags

  • All other flags correspond to and override their config file equivalents…

I see this as something that could help debugging Terser itself

I think that this could also be implemented in Terser, but it can do so independently, whether in the CLI only or also via API like Babel does. If in the end both projects implement, the upper-most project can drop it.


If this doesn't belong here, maybe webpack-cli could this:

  • it knows where webpack.config.js is
  • it already calls TerserPlugin internally
  • it already accepts flags so --no-terserrc is easy to implement
  • it would match Parcel’s behavior, making it easier for the community to come around the same standard

@alexander-akait
Copy link
Member

Ideally tools should solve this on own side, because configurations very different and can be changed between releases, so await terser.getConfigFile() should help other tools integrate this, like do babel/eslint/stylelint/prettier/etc

@masx200
Copy link

masx200 commented Aug 23, 2022

I think we should call it "terser.config.js".

@masx200
Copy link

masx200 commented Aug 23, 2022

@masx200
Copy link

masx200 commented Aug 23, 2022

function createTerserPlugin() {
    const terserconfig: {
        // minify: typeof TerserPlugin.terserMinify;
        parallel: boolean;
        terserOptions: MinifyOptions;
    } = {
        // minify: TerserPlugin.terserMinify,
        terserOptions: {
            ecma: 5,
            parse: {
                ecma: 2015,
            },
            compress: {
                // warnings: !1,
                comparisons: !1,
                inline: 2,
                drop_console: true,
                drop_debugger: true,
                pure_funcs: ["console.log"],
            },
            mangle: {
                safari10: !0,
            },
            output: {
                ecma: 5,
                comments: !1,
                ascii_only: !0,
            },
        },
        parallel: !0,
    };
    if (fs.existsSync(path.resolve(__dirname, "terser.config.js"))) {
        const mergedconfig = merge.recursive(
            true,
            terserconfig,
            require(path.resolve(__dirname, "terser.config.js"))
        ) as typeof terserconfig;
        //@ts-ignore
        return new TerserPlugin<MinifyOptions>(mergedconfig);
    }
    //@ts-ignore
    return new TerserPlugin<MinifyOptions>(terserconfig);
}

@masx200
Copy link

masx200 commented Aug 23, 2022

"babel.config.js"

"tsconfig.json"

"postcss.config.js"
"terser.config.js"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants