Skip to content

[Frontend] Adding info about comments #19463

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

Merged
merged 1 commit into from
Jan 30, 2024

Conversation

ThomasLandauer
Copy link
Contributor

Page: https://symfony.com/doc/6.4/frontend.html

@carsonbot carsonbot added this to the 6.4 milestone Jan 28, 2024
@smnandre
Copy link
Member

For maximal fairness, what do you think adding a link to the needed PostCSS install documentation (as it's done with AssetMapper SASS/Tailwing) ?

-> https://symfony.com/doc/4.x/frontend/encore/postcss.html

@ThomasLandauer
Copy link
Contributor Author

First (and foremost): Removing comments is an important topic! To backend devs (i.e. typical Symfony users) it might not be obvious that all their comments will be public in the end. So I think that there should be an own heading at https://symfony.com/doc/6.4/frontend/asset_mapper.html about comments.

But what to write there? Some counter-arguments against PostCSS:

  • It only covers CSS files, not JavaScript.
  • Knowing the JavaScript community, I'm sure it's not the only possible tool for this purpose ;-)
  • Giving detailed instructions might become a maintenance nightmare soon. What was the reason for abandoning https://symfony.com/doc/4.x/frontend/encore/postcss.html ?

But what about just adding a general information (to make people aware), followed by a list of links to some appropriate tools? Something like:

Since AssetMapper does not modify your files, all comments in CSS and JavaScript files will remain in the deployed files and become readable by the public. To remove comments, you might want to take a look at the following tools:

Ping @weaverryan @javiereguiluz - what do you think?

@smnandre
Copy link
Member

@ThomasLandauer

I was only referencing the fact Encore needs another tool to remove comments from CSS files... so the row you suggested in your PR could be 100% objective with that precision.

@ThomasLandauer
Copy link
Contributor Author

Oh sorry - I should have opened a separate issue then ;-)

So what you're saying, is:

  • Encore removes comments from JavaScript files out of the box
  • To remove comments from CSS files too, you need to set up a tool like PostCSS
    Right?
    => Hm, not so easy to get across in one table cell ;-) => Maybe a separate row for JavaScript and CSS?

@smnandre
Copy link
Member

What I'm saying is: if there were a row concerning comment removal in this table, it should not create the false impression that Encore trims comments from CSS files without additional tools ;)

@ThomasLandauer
Copy link
Contributor Author

Yeah, thanks, I now changed the table. What do you think?

@javiereguiluz
Copy link
Member

Thank Thomas!

@ThomasLandauer ThomasLandauer deleted the patch-6 branch January 30, 2024 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants