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(gatsby): fix some css HMR edge cases #29839

Merged
merged 4 commits into from
Mar 1, 2021
Merged

fix(gatsby): fix some css HMR edge cases #29839

merged 4 commits into from
Mar 1, 2021

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Feb 28, 2021

Description

Add test cases for issues described in:

Current status (tests for removing imports only pass because adding imports doesn't do anything :) ):
Screenshot 2021-02-28 at 23 24 36

With this veeeeeery hacky fix workaround:
Screenshot 2021-02-28 at 23 52 51

Related Issues

[ch25603]
[ch25604]

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Feb 28, 2021
@pieh pieh force-pushed the v3/css-hmr-edge-cases branch from c5afaf5 to 5cfc027 Compare February 28, 2021 21:51
@pieh pieh added topic: webpack/babel Webpack or babel and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Feb 28, 2021

apply(compiler: Compiler): void {
compiler.hooks.thisCompilation.tap(this.name, compilation => {
compilation.hooks.fullHash.tap(this.name, () => {
Copy link
Contributor Author

@pieh pieh Feb 28, 2021

Choose a reason for hiding this comment

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

this entire hack is largely based on checking what HMR internal webpack plugin does ( https://github.com/webpack/webpack/blob/04a7d052b5c04628257ec23244f234622c7f394c/lib/HotModuleReplacementPlugin.js#L323-L411 ) and "tricking it" into thinking that blank.css file we import (

import "./blank.css"
) is changed when:
a) there is new css module or hash of already existing one changed (we set anyCssChanged to true)
b) when css module that was in previous compilation is no longer there (that's combo of seenCssInThisCompilation and this.previouslySeenCss)

Comment on lines +3 to +14
/**
* This is total hack that is meant to handle:
* - https://github.com/webpack-contrib/mini-css-extract-plugin/issues/706
* - https://github.com/webpack-contrib/mini-css-extract-plugin/issues/708
* The way it works it is looking up what HotModuleReplacementPlugin checks internally
* and tricks it by checking up if any modules that uses mini-css-extract-plugin
* changed or was newly added and then modifying blank.css hash.
* blank.css is css module that is used by all pages and is there from the start
* so changing hash of that _should_ ensure that:
* - when new css is imported it will reload css
* - when css imported by not loaded (by runtime) page template changes it will reload css
*/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of this does also rely on fact that we extract css to single commons.css file. To be fair - fact that we do that is also part of the problem (particularly issue about not visited modules that import css)

@pieh pieh marked this pull request as ready for review February 28, 2021 22:55
Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

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

Looks good, added 2 small comments

Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

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

Thanks, this looks wonderful! 👍

@wardpeet wardpeet changed the title fix(hmr): fix some css HMR edge cases fix(gatsby): fix some css HMR edge cases Mar 1, 2021
@pieh pieh merged commit 52facaf into master Mar 1, 2021
@pieh pieh deleted the v3/css-hmr-edge-cases branch March 1, 2021 10:46
vladar pushed a commit that referenced this pull request Mar 1, 2021
* test(e2e-development-runtime): add test cases for various edge cases related to css HMR

* hackity hack

* Update packages/gatsby/src/utils/webpack/force-css-hmr-for-edge-cases.ts

Co-authored-by: Ward Peeters <ward@coding-tech.com>

* add some comments with explanation

Co-authored-by: Ward Peeters <ward@coding-tech.com>
(cherry picked from commit 52facaf)
vladar pushed a commit that referenced this pull request Mar 1, 2021
* test(e2e-development-runtime): add test cases for various edge cases related to css HMR

* hackity hack

* Update packages/gatsby/src/utils/webpack/force-css-hmr-for-edge-cases.ts

Co-authored-by: Ward Peeters <ward@coding-tech.com>

* add some comments with explanation

Co-authored-by: Ward Peeters <ward@coding-tech.com>
(cherry picked from commit 52facaf)

Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: webpack/babel Webpack or babel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants