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

Better tree shaking for prerender_multi_page_bundled() CSS #27

Closed
dgp1130 opened this issue Feb 18, 2021 · 2 comments
Closed

Better tree shaking for prerender_multi_page_bundled() CSS #27

dgp1130 opened this issue Feb 18, 2021 · 2 comments
Labels
feature New feature or request
Milestone

Comments

@dgp1130
Copy link
Owner

dgp1130 commented Feb 18, 2021

Spin off from #1.

Currently, prerender_multi_page_bundled() is somewhat suboptimal in generating bundled CSS. Every page declares some CSS files it needs, each of which imports its own graph of dependencies. However, all the CSS modules for every page are combined into a single entry point, which is then bundled by PostCSS. That singular bundle is then used for all generated HTML pages.

This means that if a single page imports module A, and no other page uses A, they will all still bundle A, download it, and apply it. This is additional bundle size which is not desirable or useful to users. It also means devs need to write their CSS to be conservative about content on the page, as it may be applied to unrelated pages. If styles are appropriately isolated, this won't have too much impact, but generic styles like p could totally break other pages they should never have been included with.

I believe this is a limitation of postcss_binary(), but I'll need to do some research to confirm. If so, we will likely need to make a FR to add support for accepting execution-time entry points.

@dgp1130
Copy link
Owner Author

dgp1130 commented Mar 5, 2021

Finally got around to filing an issue with rules_postcss: bazelbuild/rules_postcss#63

@dgp1130
Copy link
Owner Author

dgp1130 commented May 6, 2022

I believe this is now obsolete given that global styles have been removed and all styles are inlined (#47). Since inlined styles are bundled at the component level, they are known at analysis time. There may be some build time benefit to delaying this work until the page is fully rendered and would run into this problem again, but I don't think this is an immediate problem to worry about.

@dgp1130 dgp1130 closed this as completed May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant