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

Change the optimization minify option to allow removing whitespaces but keep comments #26427

Closed
CarlosTorrecillas opened this issue Nov 20, 2023 · 14 comments

Comments

@CarlosTorrecillas
Copy link
Contributor

Which @angular/* package(s) are relevant/related to the feature request?

No response

Description

At the moment, as far as I know and as per the docs, the minify option accepts only a boolean and does everything to reduce the size of the final CSS (removing excess of whitespaces and comments). This can become a problem if you run a postbuild task that removes the unused CSS using PurgeCSS for example. Since purge looks for comments such as /* purgecss start ignore */ it won't find them and therefore it will remove the css block

Proposed solution

Extend the minify option to accept either a boolean (right now) or two properties: whitespaces and comments so we are able to tweak accordingly

Alternatives considered

Run another postbuild task that will remove the whitespaces "manually" after the CSS optimization

@JeanMeche JeanMeche transferred this issue from angular/angular Nov 20, 2023
@alan-agius4
Copy link
Collaborator

alan-agius4 commented Nov 20, 2023

In this case, you need to use CSS special comments, /*! comment here */ which CSS minifiers will not remove.

See: https://github.com/FullHuman/purgecss/blob/main/docs/safelisting.md#gotchas

@alan-agius4 alan-agius4 closed this as not planned Won't fix, can't repro, duplicate, stale Nov 20, 2023
@CarlosTorrecillas
Copy link
Contributor Author

Thank you very much. I think it would be good to get it in the minify reference section just to be aware that you can have comments using the syntax you described

@CarlosTorrecillas
Copy link
Contributor Author

@alan-agius4 The trouble here is that PurgeCSS is not able to pick the comment like that (/*! comment here */) as it seems it needs exaclty /* purgecss start ignore */.

I guess under this circumstances perhaps you can reconsider the feature request? I think that would be useful for people who suddenly not see the expected behaviour, users that rely on comments being the exact same way as described on third party libraries docs for example

@alan-agius4
Copy link
Collaborator

Based on their docs (linked above) , purgeCSS does support the mentioned comment format.

@CarlosTorrecillas
Copy link
Contributor Author

Yeah, I just saw. I'm getting in touch because it seems there is a bug

@CarlosTorrecillas
Copy link
Contributor Author

Hi again @alan-agius4 , I have been doing some tests disabling the Purge step to see what the initial output would be. It seems that when you have multiple files (SCSS files) that are combined into the style bundle some of the comments get lost - this is on my web app, newly migrated to Angular 17 today.

On a brand new application (doing ng new) the comments simply don't appear. I guess I can create an issue with that? before doing it, is it something you could quickly verify on your side?

@alan-agius4
Copy link
Collaborator

That is expected with esbuild and is per design. Esbuild will remove any comments in JS and CSS unless they are "special".

See evanw/esbuild#221 for more context.

@CarlosTorrecillas
Copy link
Contributor Author

OK - Then the comments disappearing on a brand new Angular 17 is expected. For new applications or for applications that we want to migrate, is there any example for configuring comments so that we can still use tools that rely on comments such as PurgeCSS?

@alan-agius4
Copy link
Collaborator

Esbuild by design does not allow standard comments to be retained, this includes all application both new and existing will have standard comments dropped. This also includes the Webpack build pipeline which under the hood esbuild is used as an JS and CSS minifier and optimizer.

Typically tools that rely on comments support In this case the special /*! comment */, which esbuild and other minifiers will not delete.

@CarlosTorrecillas
Copy link
Contributor Author

CarlosTorrecillas commented Nov 20, 2023

Just to clarify, esbuild WILL delete even "special" comments like /*! comment */. Is that what you are saying? So in theory there is no way to get around it at the moment

@alan-agius4
Copy link
Collaborator

Esbuild like other minifiers will not delete special comments.

@CarlosTorrecillas
Copy link
Contributor Author

I will create the repro steps since I can reproduce the behavior that removes them

@CarlosTorrecillas
Copy link
Contributor Author

Created this #26432 so that you can reproduce it

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants