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

chore: Extract CSS instead of injecting #6026

Merged
merged 1 commit into from
Jul 12, 2024
Merged

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Jul 10, 2024

📝 Summary

This will create a CSS file per entry point instead of injecting the CSS. Dynamic chunks will still import their CSS by them self.

Alternative for #6025

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests
  • Documentation (README or documentation) has been updated or is not required

This will create a CSS file per entry point instead of injecting the CSS.
Dynamic chunks will still import their CSS by them self.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@juliushaertl
Copy link
Member

Is there any benefit or downside comparing the two options form here and #6025?

@susnux
Copy link
Contributor Author

susnux commented Jul 11, 2024

Extracting:

  • Smaller JS assets as the CSS is not included, this will significantly reduce the browsers JS parsing time
  • Let the browser handle styles and not rely on the injection logic (this means we utilize addStyle so we support nonces on them e.g. if we in the future forbid unsafe styles (CSP)

Injecting:

  • No need to care where the scripts are used the styles are always injected automatically

@juliushaertl
Copy link
Member

Thanks, then I'd say extracting is the most sane.

@juliushaertl juliushaertl merged commit 98af8c4 into main Jul 12, 2024
61 of 64 checks passed
@juliushaertl juliushaertl deleted the chore/extract-css branch July 12, 2024 07:16
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.

2 participants