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
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 161 additions & 11 deletions e2e-tests/development-runtime/cypress/integration/styling/plain-css.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)`)
})
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.not-visited-plain-css-not-imported-initially {
color: red;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.not-visited-plain-css-test {
color: red;
}
Original file line number Diff line number Diff line change
@@ -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 (
<>
<p>
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
</p>
<p>css imported by this template is tested in `./plain-css.js` page</p>
</>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.plain-css-not-imported-initially {
color: red;
}
25 changes: 23 additions & 2 deletions e2e-tests/development-runtime/src/pages/styling/plain-css.js
Original file line number Diff line number Diff line change
@@ -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 (
<div data-testid="styled-element" className="plain-css-test">
test
<div style={{ color: `black` }}>
<div data-testid="styled-element" className="plain-css-test">
test
</div>
<div
data-testid="styled-element-that-is-not-styled-initially"
className="plain-css-not-imported-initially"
>
test
</div>
<div
data-testid="styled-element-by-not-visited-template"
className="not-visited-plain-css-test"
>
test
</div>
<div
data-testid="styled-element-that-is-not-styled-initially-by-not-visited-template"
className="not-visited-plain-css-not-imported-initially "
>
test
</div>
</div>
)
}
2 changes: 2 additions & 0 deletions packages/gatsby/src/utils/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -217,6 +218,7 @@ module.exports = async (
configPlugins = configPlugins
.concat([
plugins.fastRefresh({ modulesThatUseGatsby }),
new ForceCssHMRForEdgeCases(),
plugins.hotModuleReplacement(),
plugins.noEmitOnErrors(),
plugins.eslintGraphqlSchemaReload(),
Expand Down
97 changes: 97 additions & 0 deletions packages/gatsby/src/utils/webpack/force-css-hmr-for-edge-cases.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
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
*/
Comment on lines +3 to +14
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)

export class ForceCssHMRForEdgeCases {
private name: string
private originalBlankCssHash: string
private blankCssKey: string
private hackCounter = 0
private previouslySeenCss: Set<string> = new Set<string>()

constructor() {
this.name = `ForceCssHMRForEdgeCases`
}

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)

const chunkGraph = compilation.chunkGraph
const records = compilation.records

if (!records.chunkModuleHashes) {
return
}

const seenCssInThisCompilation = new Set<string>()

let anyCssChanged = 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)
this.previouslySeenCss.delete(key)

const hash = getModuleHash(module)
if (records.chunkModuleHashes[key] !== hash) {
anyCssChanged = true
}
}
}
}
}

if (
(anyCssChanged || this.previouslySeenCss.size > 0) &&
pieh marked this conversation as resolved.
Show resolved Hide resolved
this.originalBlankCssHash &&
this.blankCssKey
) {
records.chunkModuleHashes[this.blankCssKey] =
this.originalBlankCssHash + this.hackCounter++
pieh marked this conversation as resolved.
Show resolved Hide resolved
}

this.previouslySeenCss = seenCssInThisCompilation
})
})
}
}