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

Migrate to csso (has blocker) #38

Closed
DanielRuf opened this issue Apr 20, 2020 · 16 comments
Closed

Migrate to csso (has blocker) #38

DanielRuf opened this issue Apr 20, 2020 · 16 comments
Assignees
Projects

Comments

@DanielRuf
Copy link
Contributor

Blocker: #27 (comment)

@alexander-akait
Copy link
Collaborator

alexander-akait commented Aug 18, 2020

@DanielRuf just interesting - why do not migrate on cssnano? Most of developers uses cssnano use default minimizer, it is better supported and has a lot of improvements.

Also I find the problem in this - for example I want to minify my css using cssnano, so I installed cssnano, when I want to minify HTML, so I installed html-minifier-terser, but now I have clean-css in my node_modules, but I don't want to have it, because I never use it.

Solutions - remove clean-css from deps https://github.com/terser/html-minifier-terser/blob/master/package.json#L60 and provide the developer to choose css minimizer

/cc @fabiosantoscode

Ideally html minimizer should minimize only HTML and provide the developer ability to setup JS and CSS minimizer.

@alexander-akait
Copy link
Collaborator

This problem has existed for so long that I have already lost confidence that it will someday be solved.

@alexander-akait
Copy link
Collaborator

alexander-akait commented Aug 18, 2020

Please do not bind tools so tightly, this creates problems that may never be resolved - some tools can be deprecated, some of them abandoned, some of them have a lot of bugs.

@DanielRuf
Copy link
Contributor Author

DanielRuf commented Aug 18, 2020

Also I find the problem in this - for example I want to minify my css using cssnano, so I installed cssnano, when I want to minify HTML, so I installed html-minifier-terser, but now I have clean-css in my node_modules, but I don't want to have it, because I never use it.

We can maybe make these optionalDependencies as these are optin (minimize css). Regarding cssnano, I do not know if this covers the features that we need, like a comment which prevents minification.

When the users have the option to use a different css minification package, then we have to remove the parts in the documentation that mention the comments to skip / prevent it which is from clean-css - and a few more parts too including tests.

Ideally html minimizer should minimize only HTML and provide the developer ability to setup JS and CSS minimizer.

Sure but the original html-minifier used uglify-js and I guess this was a requested feature. Also people request more like #34 so they expect a certain amount of features and parts that are minified. Removing terser makes not much sense imho. Same for any css minifier. Also html-minifier is available as single standalone script and also as CLI to easily compress chunks of data. So for now we should avoid problems by removing the default minifiers.

See kangax/html-minifier#56

This problem has existed for so long that I have already lost confidence that it will someday be solved.

Which exact problem?

Please do not bind tools so tightly, this creates problems that may never be resolved - some tools can be deprecated, some of them abandoned, some of them have a lot of bugs.

This happens with every piece of software.

@alexander-akait
Copy link
Collaborator

We can maybe make these optionalDependencies as these are optin (minimize css). Regarding cssnano, I do not know if this covers the features that we need, like a comment which prevents minification.

When the users have the option to use a different css minification package, then we have to remove the parts in the documentation that mention the comments to skip / prevent it which is from csso - and a few more parts too including tests.

It will be great and solve a lot of problems, maybe event peerDependencies and peerDependenciesMeta.

Features provided minimizer can be different, some of them can be always safe, so you even don't need comments to skip / prevent minification, some of them can be even don't touch unnecessary stuff, for example I can use postcss with autoprefixer so nothing to remove (just note - it is example, if you need I can provide repo with example how it is work with html-minifier-terser). So rely on JS and CSS minimizer features is bad idea, it is out of scope HTML minimizer. HTML minimizer should provide adapter to any tool, for us we already do it - you can use custom minifyCSS function. But we still install clean-css package by default, this increases the size of the dependencies that will never be used.

Which exact problem?

The fact that the tools are so binded that we cannot change parts of them, this kills one of the important properties of programming - modularity.

This happens with every piece of software.

And yes and no, for example webpack has hooks to solve it, babel/postcss have parsers/plugins and it is great features. Here we don't have hooks/plugins, but we have options, and it would be great to give the opportunity setup own options.

We even safe 0CJS, like:

function getMinifyCSS(options) { 
  if (typeof options.minifyCSS === 'function') { 
    return options.minifyCSS;
  }

  let minifyCSS;

  try {
    minifyCSS = require('clean-css')
  } catch (error) { 
    throw new Error('Please install `clean-css` to minimize CSS');
  }

  return minifyCSS;
}

@alexander-akait
Copy link
Collaborator

See kangax/html-minifier#56

It is 2013 year 😞 We are in 2020 and a lot has changed

@DanielRuf
Copy link
Contributor Author

It is 2013 year 😞 We are in 2020 and a lot has changed

And not every consumer is webpack or uses your loaders and so on ;-)

Users expect that all available bundles (browser, Node and CLI) work the same.

So I will not and do not plan to remove these dependencies.

This makes more sense in some html-minifier-lite version (without terser and clean-css) which would be a completely different endproduct - with a different name and not under the terser org probably as there would be no terser. And this could be used by html-minifier-terser (drop in terser and clean-css / csso).

I thought you wanted to fork html-minifier(-terser) and change it to your needs?

@DanielRuf
Copy link
Contributor Author

I akso fail to see how the current discussion helps with the csso migration.

Please stay on topic. Otherwise I have to lock issues.

If there should be a version which does not minify css and js (which makes sense in some way), this would be a completely new package and a completely different topic. And this should be discussed in a separate issue. But this will not happen in html-minifier-terser.

Also because minification of css, js and more was a requested feature by many users for a very long time and this won't change in the near future, and there is a whole ecosystem outside of the webpack world which relies on this package.

Please respect that.
A separate package with a new name is the right approach.

@alexander-akait
Copy link
Collaborator

alexander-akait commented Aug 18, 2020

I thought you wanted to fork html-minifier(-terser) and change it to your needs?

still hope I can show you architectural problems that will help the whole community. But for some reason you don't want to hear me. What is the real reason for keeping them in dependencies? No extra installation? But this advantage poses more problems than it solves:

  • extra dependencies
  • abandoned/packages packages create bugs and only one solution - fully disable minification, most of new CSS syntax is not supported by clean-css
  • impossible to use new version because they are hardcoded in package.json, we can't use terser@5, you don't see the benefits of upgrading, but i see them - https://github.com/terser/terser/blob/master/CHANGELOG.md, many features can be used for modern browsers, and some of them I already use - import.meta,but i cant use this package, html-minifier died because of this approach, developers can't use terser instead uglify-js, and right now the situation is similar
  • no flexibility
  • extra size of package itself

I can continue, speak - Fork it!, it is solutions, I never answer like that and I would never answer you like that

@alexander-akait
Copy link
Collaborator

alexander-akait commented Aug 18, 2020

Also because minification of css, js and more was a requested feature by many users for a very long time and this won't change in the near future, and there is a whole ecosystem outside of the webpack world which relies on this package.

This is exactly what I am telling you, I did not say a word here about webpack (just about hooks, same say about postcss and babel), I just want to use cssnano and latest terser version

Please stay on topic. Otherwise I have to lock issues.

Thanks for the threats, I appreciate your desire to hear someone else's opinion

@DanielRuf
Copy link
Contributor Author

just want to use cssnano

See my previous comments regarding this and csso. I might change my decision but so far csso provides what we need here. The rest is not relevant for this issue.

@DanielRuf
Copy link
Contributor Author

DanielRuf commented Aug 18, 2020

Not sure if cssnano is even an option at all at this time:

Bildschirmfoto 2020-08-18 um 17 45 54

Oh and I see where it might come from that you want to use cssnano: https://github.com/cssnano/cssnano/releases

Bildschirmfoto 2020-08-18 um 17 49 17

@alexander-akait
Copy link
Collaborator

@DanielRuf we use nightly version because wait postcss@8

@alexander-akait
Copy link
Collaborator

Unfortunately, I see your position - not to solve these architecturally problems, it's a pity. I really hoped that we could get rid of multiple same tools and focus on the one - bugs/features/docs/etc. Not wanting to listen to someone else's opinion is a bad habit in OSS, sorry for wasting your time

@DanielRuf
Copy link
Contributor Author

I think it's best we stay with clean-css and fork / improve it. I took a closer look at the code and this should be ok and we should be able to resolve the current issues with clean-css. So no need to migrate to csso or a different solution right now. Other solutions might produce new problems (ignoring markup with a comment, different outcome and different features, ...).

development automation moved this from To do to Done Aug 18, 2020
@alexander-akait
Copy link
Collaborator

No, clean-css is not supported new CSS syntax, so it is broken and not maintained clean-css/clean-css#1105

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

No branches or pull requests

2 participants