From 52facaf7c5bf377cbec42d9d9f18832751a429a1 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Mon, 1 Mar 2021 11:46:00 +0100 Subject: [PATCH] fix(gatsby): fix some css HMR edge cases (#29839) * 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 * add some comments with explanation Co-authored-by: Ward Peeters --- .../cypress/integration/styling/plain-css.js | 172 ++++++++++++++++-- ...sited-plain-css-not-imported-initially.css | 3 + .../pages/styling/not-visited-plain-css.css | 3 + .../pages/styling/not-visited-plain-css.js | 17 ++ .../plain-css-not-imported-initially.css | 3 + .../src/pages/styling/plain-css.js | 25 ++- packages/gatsby/src/utils/webpack.config.js | 2 + .../webpack/force-css-hmr-for-edge-cases.ts | 106 +++++++++++ 8 files changed, 318 insertions(+), 13 deletions(-) create mode 100644 e2e-tests/development-runtime/src/pages/styling/not-visited-plain-css-not-imported-initially.css create mode 100644 e2e-tests/development-runtime/src/pages/styling/not-visited-plain-css.css create mode 100644 e2e-tests/development-runtime/src/pages/styling/not-visited-plain-css.js create mode 100644 e2e-tests/development-runtime/src/pages/styling/plain-css-not-imported-initially.css create mode 100644 packages/gatsby/src/utils/webpack/force-css-hmr-for-edge-cases.ts diff --git a/e2e-tests/development-runtime/cypress/integration/styling/plain-css.js b/e2e-tests/development-runtime/cypress/integration/styling/plain-css.js index da189034d8150..db549a6002971 100644 --- a/e2e-tests/development-runtime/cypress/integration/styling/plain-css.js +++ b/e2e-tests/development-runtime/cypress/integration/styling/plain-css.js @@ -7,29 +7,179 @@ after(() => { }) describe(`styling: plain css`, () => { - beforeEach(() => { + it(`initial styling is correct`, () => { cy.visit(`/styling/plain-css`).waitForRouteChange() - }) - it(`initial styling is correct`, () => { cy.getTestElement(`styled-element`).should( `have.css`, `color`, `rgb(255, 0, 0)` ) - }) - it(`updates on change`, () => { - cy.exec( - `npm run update -- --file src/pages/styling/plain-css.css --replacements "red:blue" --exact` + cy.getTestElement(`styled-element-that-is-not-styled-initially`).should( + `have.css`, + `color`, + `rgb(0, 0, 0)` ) - cy.waitForHmr() - - cy.getTestElement(`styled-element`).should( + cy.getTestElement(`styled-element-by-not-visited-template`).should( `have.css`, `color`, - `rgb(0, 0, 255)` + `rgb(255, 0, 0)` ) + + cy.getTestElement( + `styled-element-that-is-not-styled-initially-by-not-visited-template` + ).should(`have.css`, `color`, `rgb(0, 0, 0)`) + }) + + describe(`changing styles/imports imported by visited template`, () => { + it(`updates on already imported css file change`, () => { + // we don't want to visit page for each test - we want to visit once and then test HMR + cy.window().then(win => { + cy.spy(win.console, `log`).as(`hmrConsoleLog`) + }) + + cy.exec( + `npm run update -- --file src/pages/styling/plain-css.css --replacements "red:blue" --exact` + ) + + cy.waitForHmr() + + cy.getTestElement(`styled-element`).should( + `have.css`, + `color`, + `rgb(0, 0, 255)` + ) + }) + + it(`importing new css file result in styles being applied`, () => { + // we don't want to visit page for each test - we want to visit once and then test HMR + cy.window().then(win => { + cy.spy(win.console, `log`).as(`hmrConsoleLog`) + }) + + cy.exec( + `npm run update -- --file src/pages/styling/plain-css.js --replacements "// UNCOMMENT-IN-TEST:/* IMPORT-TO-COMMENT-OUT-AGAIN */" --exact` + ) + + cy.waitForHmr() + + cy.getTestElement(`styled-element-that-is-not-styled-initially`).should( + `have.css`, + `color`, + `rgb(255, 0, 0)` + ) + }) + + it(`updating newly imported css file result in styles being applied`, () => { + // we don't want to visit page for each test - we want to visit once and then test HMR + cy.window().then(win => { + cy.spy(win.console, `log`).as(`hmrConsoleLog`) + }) + + cy.exec( + `npm run update -- --file src/pages/styling/plain-css-not-imported-initially.css --replacements "red:green" --exact` + ) + + cy.waitForHmr() + + cy.getTestElement(`styled-element-that-is-not-styled-initially`).should( + `have.css`, + `color`, + `rgb(0, 128, 0)` + ) + }) + + it(`removing css import results in styles being removed`, () => { + // we don't want to visit page for each test - we want to visit once and then test HMR + cy.window().then(win => { + cy.spy(win.console, `log`).as(`hmrConsoleLog`) + }) + + cy.exec( + `npm run update -- --file src/pages/styling/plain-css.js --replacements "/* IMPORT-TO-COMMENT-OUT-AGAIN */:// COMMENTED-AGAIN" --exact` + ) + + cy.waitForHmr() + + cy.getTestElement(`styled-element-that-is-not-styled-initially`).should( + `have.css`, + `color`, + `rgb(0, 0, 0)` + ) + }) + }) + + describe(`changing styles/imports imported by NOT visited template`, () => { + it(`updates on already imported css file change by not visited template`, () => { + // we don't want to visit page for each test - we want to visit once and then test HMR + cy.window().then(win => { + cy.spy(win.console, `log`).as(`hmrConsoleLog`) + }) + + cy.exec( + `npm run update -- --file src/pages/styling/not-visited-plain-css.css --replacements "red:blue" --exact` + ) + + cy.waitForHmr() + + cy.getTestElement(`styled-element-by-not-visited-template`).should( + `have.css`, + `color`, + `rgb(0, 0, 255)` + ) + }) + + it(`importing new css file result in styles being applied`, () => { + // we don't want to visit page for each test - we want to visit once and then test HMR + cy.window().then(win => { + cy.spy(win.console, `log`).as(`hmrConsoleLog`) + }) + + cy.exec( + `npm run update -- --file src/pages/styling/not-visited-plain-css.js --replacements "// UNCOMMENT-IN-TEST:/* IMPORT-TO-COMMENT-OUT-AGAIN */" --exact` + ) + + cy.waitForHmr() + + cy.getTestElement( + `styled-element-that-is-not-styled-initially-by-not-visited-template` + ).should(`have.css`, `color`, `rgb(255, 0, 0)`) + }) + + it(`updating newly imported css file result in styles being applied`, () => { + // we don't want to visit page for each test - we want to visit once and then test HMR + cy.window().then(win => { + cy.spy(win.console, `log`).as(`hmrConsoleLog`) + }) + + cy.exec( + `npm run update -- --file src/pages/styling/not-visited-plain-css-not-imported-initially.css --replacements "red:green" --exact` + ) + + cy.waitForHmr() + + cy.getTestElement( + `styled-element-that-is-not-styled-initially-by-not-visited-template` + ).should(`have.css`, `color`, `rgb(0, 128, 0)`) + }) + + it(`removing css import results in styles being removed`, () => { + // we don't want to visit page for each test - we want to visit once and then test HMR + cy.window().then(win => { + cy.spy(win.console, `log`).as(`hmrConsoleLog`) + }) + + cy.exec( + `npm run update -- --file src/pages/styling/not-visited-plain-css.js --replacements "/* IMPORT-TO-COMMENT-OUT-AGAIN */:// COMMENTED-AGAIN" --exact` + ) + + cy.waitForHmr() + + cy.getTestElement( + `styled-element-that-is-not-styled-initially-by-not-visited-template` + ).should(`have.css`, `color`, `rgb(0, 0, 0)`) + }) }) }) diff --git a/e2e-tests/development-runtime/src/pages/styling/not-visited-plain-css-not-imported-initially.css b/e2e-tests/development-runtime/src/pages/styling/not-visited-plain-css-not-imported-initially.css new file mode 100644 index 0000000000000..c1fa8bf1b883c --- /dev/null +++ b/e2e-tests/development-runtime/src/pages/styling/not-visited-plain-css-not-imported-initially.css @@ -0,0 +1,3 @@ +.not-visited-plain-css-not-imported-initially { + color: red; +} diff --git a/e2e-tests/development-runtime/src/pages/styling/not-visited-plain-css.css b/e2e-tests/development-runtime/src/pages/styling/not-visited-plain-css.css new file mode 100644 index 0000000000000..18d98b2f4b841 --- /dev/null +++ b/e2e-tests/development-runtime/src/pages/styling/not-visited-plain-css.css @@ -0,0 +1,3 @@ +.not-visited-plain-css-test { + color: red; +} diff --git a/e2e-tests/development-runtime/src/pages/styling/not-visited-plain-css.js b/e2e-tests/development-runtime/src/pages/styling/not-visited-plain-css.js new file mode 100644 index 0000000000000..5f601d51959f0 --- /dev/null +++ b/e2e-tests/development-runtime/src/pages/styling/not-visited-plain-css.js @@ -0,0 +1,17 @@ +import * as React from "react" + +import "./not-visited-plain-css.css" +// UNCOMMENT-IN-TEST import "./not-visited-plain-css-not-imported-initially.css" + +export default function PlainCss() { + return ( + <> +

+ This content doesn't matter - we never visit this page in tests - but + because we generate single global .css file, we want to test changing + css files imported by this module (and also adding new css imports).css +

+

css imported by this template is tested in `./plain-css.js` page

+ + ) +} diff --git a/e2e-tests/development-runtime/src/pages/styling/plain-css-not-imported-initially.css b/e2e-tests/development-runtime/src/pages/styling/plain-css-not-imported-initially.css new file mode 100644 index 0000000000000..75370b46f15e2 --- /dev/null +++ b/e2e-tests/development-runtime/src/pages/styling/plain-css-not-imported-initially.css @@ -0,0 +1,3 @@ +.plain-css-not-imported-initially { + color: red; +} diff --git a/e2e-tests/development-runtime/src/pages/styling/plain-css.js b/e2e-tests/development-runtime/src/pages/styling/plain-css.js index 8e622f4e561d6..a54b139792648 100644 --- a/e2e-tests/development-runtime/src/pages/styling/plain-css.js +++ b/e2e-tests/development-runtime/src/pages/styling/plain-css.js @@ -1,11 +1,32 @@ import * as React from "react" import "./plain-css.css" +// UNCOMMENT-IN-TEST import "./plain-css-not-imported-initially.css" export default function PlainCss() { return ( -
- test +
+
+ test +
+
+ test +
+
+ test +
+
+ test +
) } diff --git a/packages/gatsby/src/utils/webpack.config.js b/packages/gatsby/src/utils/webpack.config.js index 18b21a4e15e3b..d99577b00b957 100644 --- a/packages/gatsby/src/utils/webpack.config.js +++ b/packages/gatsby/src/utils/webpack.config.js @@ -18,6 +18,7 @@ import { createWebpackUtils } from "./webpack-utils" import { hasLocalEslint } from "./local-eslint-config-finder" import { getAbsolutePathForVirtualModule } from "./gatsby-webpack-virtual-modules" import { StaticQueryMapper } from "./webpack/static-query-mapper" +import { ForceCssHMRForEdgeCases } from "./webpack/force-css-hmr-for-edge-cases" import { getBrowsersList } from "./browserslist" import { builtinModules } from "module" @@ -217,6 +218,7 @@ module.exports = async ( configPlugins = configPlugins .concat([ plugins.fastRefresh({ modulesThatUseGatsby }), + new ForceCssHMRForEdgeCases(), plugins.hotModuleReplacement(), plugins.noEmitOnErrors(), plugins.eslintGraphqlSchemaReload(), diff --git a/packages/gatsby/src/utils/webpack/force-css-hmr-for-edge-cases.ts b/packages/gatsby/src/utils/webpack/force-css-hmr-for-edge-cases.ts new file mode 100644 index 0000000000000..7f6b3257080c3 --- /dev/null +++ b/packages/gatsby/src/utils/webpack/force-css-hmr-for-edge-cases.ts @@ -0,0 +1,106 @@ +import { Compiler, Module } from "webpack" + +/** + * 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 + */ +export class ForceCssHMRForEdgeCases { + private name: string + private originalBlankCssHash: string + private blankCssKey: string + private hackCounter = 0 + private previouslySeenCss: Set = new Set() + + constructor() { + this.name = `ForceCssHMRForEdgeCases` + } + + apply(compiler: Compiler): void { + compiler.hooks.thisCompilation.tap(this.name, compilation => { + compilation.hooks.fullHash.tap(this.name, () => { + const chunkGraph = compilation.chunkGraph + const records = compilation.records + + if (!records.chunkModuleHashes) { + return + } + + const seenCssInThisCompilation = new Set() + /** + * We will get list of css modules that are removed in this compilation + * by starting with list of css used in last compilation and removing + * all modules that are used in this one. + */ + const cssRemovedInThisCompilation = this.previouslySeenCss + + let newOrUpdatedCss = false + + for (const chunk of compilation.chunks) { + const getModuleHash = (module: Module): string => { + if (compilation.codeGenerationResults.has(module, chunk.runtime)) { + return compilation.codeGenerationResults.getHash( + module, + chunk.runtime + ) + } else { + return chunkGraph.getModuleHash(module, chunk.runtime) + } + } + + const modules = chunkGraph.getChunkModulesIterable(chunk) + + if (modules !== undefined) { + for (const module of modules) { + const key = `${chunk.id}|${module.identifier()}` + + if ( + !this.originalBlankCssHash && + module.rawRequest === `./blank.css` + ) { + this.blankCssKey = key + this.originalBlankCssHash = + records.chunkModuleHashes[this.blankCssKey] + } + + const isUsingMiniCssExtract = module.loaders?.find(loader => + loader?.loader?.includes(`mini-css-extract-plugin`) + ) + + if (isUsingMiniCssExtract) { + seenCssInThisCompilation.add(key) + cssRemovedInThisCompilation.delete(key) + + const hash = getModuleHash(module) + if (records.chunkModuleHashes[key] !== hash) { + newOrUpdatedCss = true + } + } + } + } + } + + // If css file was edited or new css import was added (`newOrUpdatedCss`) + // or if css import was removed (`cssRemovedInThisCompilation.size > 0`) + // trick Webpack's HMR into thinking `blank.css` file changed. + if ( + (newOrUpdatedCss || cssRemovedInThisCompilation.size > 0) && + this.originalBlankCssHash && + this.blankCssKey + ) { + records.chunkModuleHashes[this.blankCssKey] = + this.originalBlankCssHash + String(this.hackCounter++) + } + + this.previouslySeenCss = seenCssInThisCompilation + }) + }) + } +}