This repository has been archived by the owner on Jun 2, 2022. It is now read-only.
[FSSS-143] Chore: remove unused css (spike) #244
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Preview is readyThis pull request generated a Preview👀 Preview: https://preview-244--base.preview.vtex.app |
eduardoformiga
changed the title
eduardoformiga
changed the title
eduardoformiga
force-pushed
the
chore/remove-unused-css
branch
from
b32811e
to
8ddb844
Compare
eduardoformiga
changed the title
eduardoformiga
added
documentation
1 task
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What's the purpose of this pull request?
This PR aims to apply some experiments in order to deal with critical CSS, unused CSS, code-splitting.
Edit Note:
The code from this PR was also added on f7ae9c8 on #241. so that we can solve the unused CSS problem there.
So this PR here becomes a study case that we can visit as necessary in the future :)
Context
Lighthouse (LH) started complaining about
unused css:Nowadays:
yarn build), the CSS from all pages and components are injected inside each page, minified, inside a<style>tag in the<head>. Thus, the browser avoids one more request to fetch a CSS file.yarn develop) everything is extracted into a CSS file (commons.css) and imported using<link>in the<head>. This function is the function of webpack'smini-css-extract-plugin.Problem
All the CSS is being injected into pages globally, even unused ones.
This is a known issue in Gatsby.
In the
v2version, a technique calledcode-splitting CSSwas used, I imagine that the CSS of each component/page was separate, and only what was used was imported into the pages. But this proved to be problematic #11072.So in
v3, which is the version we are using now,code splittinghas become optional and needs to be done manually and all CSS is placed globally on each page as mentioned earlier. Thus, we need to at least scope the CSS so that we don't have either style breaking or unexpected behavior. This generates a lot of unused CSS on the pages. Also known issue #28046, #21728, #18732 .Techniques and tools
The technique recommended by
web.dev:references: #1, #2, #3
There are some tools and plugins to do this. But we need to know if it matches well with the
gatsby/webpackstack and test:critical, html-critical-webpack-plugin.
Another technique is to
remove unused CSS, using something like PurgeCSS, which also has gatsby-plugin-purgecss for gatsby.There is also another technique known as
tree-shaking(#4, #5).css-modules doesn't solve our problem. I did some tests too and what it does is scope our classes well. It uses a hash for this. The final markup is a little worse to read but it solves the class collision problem. It might be a future approach, nowadays we scope using
BEM + data-attributes. Thecss-modulesis very class-focused, we would have to adapt thedata-attributes, for example. It would solve the collision problem but not the unused CSS problem.This repository lists some
css-in-jstechniques.styled-componentssolves our problem, but I don't particularly like how the final hashed markup looks. We would have to adapt a lot of things in our CSS too.I think it's worth doing some experiments and trying to adapt the rules of
webpackfrom gatsby like this. Although understanding the initial configuration that gatsby does is a bit tricky.CSS splitting
commit: 5dc155b
CSS coverage after this Webpack setup:
✅ Code splitting CSS with only the global CSS and the page CSS. Does not include the other page's CSS.
CSS splitting + Critical CSS
commit: 9d00e97
Testing critical CSS only on homepage.
inline
critical above the fold CSSand deferuncritical below the fold cssResult: it's not possible to test in the preview environment since we need the puppeteer installed.
❌ Preview Build failed:
Locally:
❌ Drawback:
<link rel="stylesheet">).✅ The critical CSS was applied inline and has a good CSS used rate, but some duplicated CSS needs to be removed yet (from the page's CSS).
CSS splitting + PurgeCSS
commit: 8b08fd3
❌ PurgeCSS does not support attribute (like
data-attributes) or complex selectors: Issue #110.✅ When we just purge the
global.scsswe gain almost nothing +- (0,5kb).CSS splitting + Custom Loaders
❌ Attempts to optimize our CSS with custom loaders like dropCSS, clean-css, CSSO, but the final result doesn't have much difference in the final result.
CSS splitting + Tree shaking.
commit: 8f1b57d
❌ Testing with
sideEffectsas true, false, and using eitherloadersor inside thepackage.jsonbroke the style or doesn't have much difference in the final result.Conclusion
Let's just keep the code splitting CSS with scoped CSS so far. This technique brings more gains without cumbersome other configs/techniques that bring less for our context.
References
https://www.notion.so/vtexhandbook/WIP-Gatsby-Webpack-Global-CSS-problem-203c96c86bcd48fb930c3d053756fda4