Skip to content
New issue

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

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

Already on GitHub? # to your account

Changing minification defaults #774

Closed
ctron opened this issue Apr 17, 2024 · 0 comments · Fixed by #775
Closed

Changing minification defaults #774

ctron opened this issue Apr 17, 2024 · 0 comments · Fixed by #775
Milestone

Comments

@ctron
Copy link
Collaborator

ctron commented Apr 17, 2024

Some folks would prefer to keep minification off by default. So, let's figure out how we can get there, in a reasonable way.

Right now, minification is on by default for --release builds. It can be disabled on a "per asset" setting via the data-no-minification attribute. Or globally, using --no-minification.

In order to make this a conscious decision, the idea is to not enable minification based on the --release flag, starting with Trunk 0.20.x, as this will be a breaking change.

The question is, how is this supposed to work? Ideally, there would some "release" profile where one could configure this. However, my feeling is that adding profiles right now is a bit too much.

I think it makes sense to have a global switch for minification (e.g. --minification, -M, build.minification). I can have three values: Never, OnRelease, Always. Internally it is Option<Minification> and defaults to None. A value of None will translate to Never when evaluating. That way we can override from the command line.

An asset can still opt-out of the while process using data-no-minification.

If possible (need to check with clap), passing just --minification (-M) would lead to Minification::Always. -M never would override any setting in Trunk.toml.

I think this way we should be able to have minification off by default, can easily turn it on from the command line or config, and still can have assets opt-out if required. Any thoughts?

Also see:

@ctron ctron added this to the 0.20.0 milestone Apr 17, 2024
ctron added a commit to ctron/trunk that referenced this issue Apr 18, 2024
ctron added a commit to ctron/trunk that referenced this issue Apr 18, 2024
ctron added a commit to ctron/trunk that referenced this issue Apr 18, 2024
BREAKING-CHANGE: This changes the default behavior of minification to
always off. It also drops the --no-minification argument in favor of a
`--minify <value>` option.

Closes: trunk-rs#774
ctron added a commit to ctron/trunk that referenced this issue Apr 18, 2024
BREAKING-CHANGE: This changes the default behavior of minification to
always off. It also drops the --no-minification argument in favor of a
`--minify <value>` option.

Closes: trunk-rs#774
ctron added a commit to ctron/trunk that referenced this issue Apr 18, 2024
BREAKING-CHANGE: This changes the default behavior of minification to
always off. It also drops the --no-minification argument in favor of a
`--minify <value>` option.

closes trunk-rs#774
ctron added a commit that referenced this issue Apr 18, 2024
BREAKING-CHANGE: This changes the default behavior of minification to
always off. It also drops the --no-minification argument in favor of a
`--minify <value>` option.

closes #774
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant