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

CSS: minify more CSS #178

Closed
tdewolff opened this issue Feb 24, 2018 · 7 comments
Closed

CSS: minify more CSS #178

tdewolff opened this issue Feb 24, 2018 · 7 comments

Comments

@tdewolff
Copy link
Owner

tdewolff commented Feb 24, 2018

Pushing #177 gave some minification regressions. There should be a list of functions that can be safely minified by just minifying the individual tokens, and functions that require more parsing.

Non-exhaustive list of functions:

Additionally, functions should be minified recursively.

rgba is an alias for rgb (and hsla for hsl), see https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#rgba(). Also check if we can rewrite into #RRGGBBAA.
Convert hsla(0,100%,50%,.5) to #ff000080. NB: percentage to 0-255 has unspecified rounding, so that only 0% -> 0 and 100% -> 255 can be determined safely (and maybe other that are integers too, such as 20% -> 51, 40% -> 102, 60% -> 153 and 80% -> 204).

Aggregate CSS declarations should minify more comprehensively (such as font, background, mask, border, outline, flex?, grid, transition?, ...), see https://developer.mozilla.org/en-US/docs/Web/CSS/font and tests in https://github.com/css/csso/tree/master/test/fixture/compress. For font remove default line-height = normal and font-weight = 400.

For background-position, we can transform [left|right|top|bottom|length|percentage] [center|50%] to [left|right|top|bottom|length|percentage]. Also left top and top left are defaults, except when either is followed by an offset (unless an offset is 0 or 0%, then we can remove it). We can always do left|top to 0, center to 50% and right|bottom to 100%. See https://drafts.csswg.org/css-backgrounds-3/#background-position What about comma separations in background-position and others?

Remove escaped newlines from strings, g:url('asd\ sdf') -> g:url(asdsdf), see https://drafts.csswg.org/css-values-3/#strings

Rewrite dimensions to shorter ones, for example 200grad -> 180deg. But also 0ms -> 0s and 500ms -> .5s or .005s -> 5ms.

Bug: .block[data-value="test" i]{color:red} removes space between test and i.

See: https://cssnano.co/optimisations/

@tdewolff tdewolff changed the title CSS: parse and minify more CSS functions CSS: parse and minify more CSS Jul 16, 2018
@tdewolff tdewolff changed the title CSS: parse and minify more CSS CSS: minify more CSS Jul 16, 2018
tdewolff added a commit that referenced this issue Jul 16, 2018
@dchenk
Copy link
Contributor

dchenk commented Jul 26, 2018

I would like to be able to turn off any of the transformation rules. Think of webpack and why it has become so popular: it's fully customizable. And I feel that this library should offer the same customization abilities.

I like to play it safe with code minification tools. Ideally for my purposes, I'd have just extra whitespace and comments removed.

Thanks. :)

@tdewolff
Copy link
Owner Author

Hi @dchenk, I unfortunately have to disagree with you here. I think the customizability of other libraries is a hindrance that introduces complexity and "too much choice". For an analogy, think of why the Apple operating system is popular: works out of the box and doesn't let you customize. This is one of the objectives, and while it might not yet always work out of the box, I try to move it towards that way. In short, I don't see why disabling some transformations and keeping others is of help to anyone. Could you explain how this would help you, and more concretely what problem you have now that can be solved with making everything customizable?

The options I do approve of, are those that involve unsafe transformations (removing decimal digits for numbers) and some concession surrounding controversial matters (see the HTML minifier). If you just want whitespace and comments removed, I think that can be programmed in ~30 lines of code.

@dchenk
Copy link
Contributor

dchenk commented Jul 26, 2018

I understand where you're coming from, @tdewolff, but it will only make this library more powerful to expose more control to the client. All defaults can stay as they are—no problem. There should be a "zero config" setup by default. But when I'm working with someone else's code, they expect that what they write will be faithfully represented in what gets sent to the user's browsers, which for me means reducing the number of uncalled-for transformations of the designer's code. The issue #180 is a great example of what goes wrong when the sole maintainer of a package makes choices without any oversight and, frankly, without sufficient unit tests for the code.

@tdewolff
Copy link
Owner Author

tdewolff commented Jul 26, 2018

I think you are out of line here. My work doesn't need any oversight, do not disrespect me. You are, however, free to add more unit tests or to convince me about design decisions / minification rules, but I expect you to properly read the code and specifications first and back-up your claims before you start finger-pointing and insulting.

@dchenk
Copy link
Contributor

dchenk commented Jul 26, 2018

@tdewolff my apologies. Didn't mean to offend you. It's your library, so have the final say.

I'd like to ask, though, are you inviting contributions? (Some maintainers of other libraries are over-protective and aggressive with people trying to contribute. I'd like to help build off your work and leverage it for my purposes if that's okay with you.)

And thank you for the great work you've already done!

@tdewolff
Copy link
Owner Author

@dchenk I do invite contributors, but be aware that considerable thought has gone into much of the code, especially regarding performance and correctness. I therefore advice to bring up suggestions / proposals instead of PRs if you intent (large) changes. Code reviews are slow and strict so as to uphold quality, performance and correctness of all edge cases!

Regarding test coverage, this is close to 100% already but improvements are always welcome.

@tdewolff tdewolff mentioned this issue Dec 10, 2018
18 tasks
@tdewolff
Copy link
Owner Author

This has been incorporated into #234 for better organisation.

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

No branches or pull requests

2 participants