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

fix(core): Disable buildin lightingcss when inject style #3254

Closed
wants to merge 5 commits into from

Conversation

witsaint
Copy link

Summary

For production builds, it is recommended to use the default behavior of Rsbuild, which extracts CSS into separate bundles to allow browsers to load CSS and JS assets in parallel.

Related Links

[Bug]: rsbuild injectStyles

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

@CLAassistant
Copy link

CLAassistant commented Aug 20, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

netlify bot commented Aug 20, 2024

Deploy Preview for rsbuild ready!

Name Link
🔨 Latest commit e67a4c7
🔍 Latest deploy log https://app.netlify.com/sites/rsbuild/deploys/66c54da0c43073000724f96b
😎 Deploy Preview https://deploy-preview-3254--rsbuild.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 72 (no change from production)
Accessibility: 97 (no change from production)
Best Practices: 100 (no change from production)
SEO: 92 (no change from production)
PWA: 60 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@chenjiahan chenjiahan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why do we need to disable builtin lightingcss when using injectStyles?

@witsaint
Copy link
Author

Can you explain why do we need to disable builtin lightingcss when using injectStyles?

1: As described in our documentation, when we open injectStyles,If you need to enable this option in production mode, please note that the inlined CSS code will not go through Rsbuild's default CSS minimizer.

2:
a: When I debugger Rsbuild,I found that there were five loaders before stylePostLoader
image

b:The latest loader is builtin lightingcss
image
The same is shown in rspack.config.web.mjs

c:I found that after using postcss with cssnano, source works fine,But by buildin lightingcss,Found a lot of \n
image

d: When I remove buildin lightingcss in loader, css compression is fine when injectStyles is turned on

@chenjiahan
Copy link
Member

Get. I believe this fix is not the optimal solution as it prevents PostCSS and LightningCSS from working together.

We can solve this problem by simply adding a minify option to Rspack's lightningcss-loader and enanbling minify when output.injectStyles is used.

@chenjiahan
Copy link
Member

Sorry, we should not change the order of lightningcss-loader and postcss-loader, this is an unexpected change.

Take Tailwind CSS as an example. Tailwind CSS it is a PostCSS plugin, it will generate some CSS code, then Lightning CSS will add vendor prefixes to those CSS properties. If we change the loader order, this will be broken.

@chenjiahan
Copy link
Member

If we need to adjust the default loader behavior, it is best to first discuss the impact of doing so.

@witsaint witsaint closed this Aug 21, 2024
@witsaint witsaint deleted the fix-injectstyle-minify branch November 27, 2024 09:52
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.

3 participants