Skip to content
This repository has been archived by the owner on Jun 2, 2022. It is now read-only.

[FSSS-214] Chore: ignoreOrder warnings from MiniCssExtractPlugin #390

Closed
wants to merge 8 commits into from

Conversation

eduardoformiga
Copy link
Member

@eduardoformiga eduardoformiga commented Mar 15, 2022

What's the purpose of this pull request?

Solve some warnings on localhost from the mini-css-extract-plugin.

How does it work?

As explained previously on #244

  • Locally, using develop mode (yarn develop), the CSSs from all pages and components on localhost are being extracted into a single CSS file (commons.css) and imported using <link> in the <head>.

Screen Shot 2022-01-17 at 13 27 24

Thus, as a result of having all CSS together, we are getting some warnings on localhost from the mini-css-extract-plugin.

  • On the other hand, in production (yarn build), as we enabled the CSS code-splitting, we have only the CSS that is being used in the pages/components. Thus optimizing performance and avoiding class collision and CSS order warnings.
    In addition, the CSS from the page being used is minified, inside a <style> tag in the <head>. Thus, the browser avoids one more request to fetch a CSS file.

So, we just have this problem locally then, and, while investigating how to deal with it, we started to test the use of CSS modules to analyze gains regarding the CSS order warnings and performance. But the result was almost the same as before.

Finally, reading some issues and threads (#1, #2, #3, #4, #5) we decide to ignore the order from the mini-css-extract-plugin once we have our CSS scoped already and it looks like this is the recommended way to avoid this problem so far.

How to test it?

Check the console tab on Devtools after running yarn develop and double-check that we don't have warnings regarding the CSS order as we had previously.
You can also check if the warnings were gone on the same console that you run the yarn develop command.

References

https://vtex-dev.atlassian.net/browse/FSSS-214
https://vtex-dev.atlassian.net/browse/FSSS-127

Checklist

  • CHANGELOG entry added

@netlify
Copy link

netlify bot commented Mar 15, 2022

Deploy Preview for basestore ready!

Name Link
🔨 Latest commit b4a803d
🔍 Latest deploy log https://app.netlify.com/sites/basestore/deploys/623ba9b65d126c0009bad9cc
😎 Deploy Preview https://deploy-preview-390--basestore.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@vtex-sites
Copy link

vtex-sites bot commented Mar 15, 2022

Preview is ready

This pull request generated a Preview

👀   Preview: https://preview-390--base.preview.vtex.app
🔬   Go deeper by inspecting the Build Logs
📝   based on commit b4a803d

@gatsby-cloud
Copy link

gatsby-cloud bot commented Mar 15, 2022

Gatsby Cloud Build Report

basestore

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 6m

Performance

Lighthouse report

Metric Score
Performance 💚 94
Accessibility 💚 100
Best Practices 💚 100
SEO 💚 93

🔗 View full report

@eduardoformiga eduardoformiga marked this pull request as ready for review March 15, 2022 22:00
Copy link
Contributor

@filipewl filipewl left a comment

Choose a reason for hiding this comment

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

It looks good to me based on what we had discussed!

@filipewl filipewl requested a review from a team March 16, 2022 18:38
@eduardoformiga eduardoformiga requested a review from a team March 18, 2022 21:28
CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Larícia Mota <[email protected]>
@eduardoformiga eduardoformiga mentioned this pull request Mar 30, 2022
1 task
@eduardoformiga
Copy link
Member Author

Close due to #434

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants