Skip to content

Commit

Permalink
Fix server CSS imports and HMR not working properly in specific condi…
Browse files Browse the repository at this point in the history
…tions (#49462)

In Hot Reloader we use `mod.resource.replace(/\\/g, '/').includes(key)`
to see if the module is the entry. However that `includes` check isn't
solid. Say the entry key is `app/path/page`, these will all be matched:

```
app/path/page.js
app/path/page.css
app/unrelated/app/path/page/another/page.js
```

This PR fixes that and added a test case. I'm unsure if this fixes the
newly reported cases in #43396.

fix NEXT-1110
  • Loading branch information
shuding committed May 9, 2023
1 parent 201ab71 commit 881d202
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 1 deletion.
8 changes: 7 additions & 1 deletion packages/next/src/server/dev/hot-reloader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,10 @@ export default class HotReloader {
const prevEdgeServerPageHashes = new Map<string, string>()
const prevCSSImportModuleHashes = new Map<string, string>()

const pageExtensionRegex = new RegExp(
`\\.(?:${this.config.pageExtensions.join('|')})$`
)

const trackPageChanges =
(
pageHashMap: Map<string, string>,
Expand Down Expand Up @@ -912,7 +916,9 @@ export default class HotReloader {
modsIterable.forEach((mod: any) => {
if (
mod.resource &&
mod.resource.replace(/\\/g, '/').includes(key)
mod.resource.replace(/\\/g, '/').includes(key) &&
// Shouldn't match CSS modules, etc.
pageExtensionRegex.test(mod.resource)
) {
// use original source to calculate hash since mod.hash
// includes the source map in development which changes
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/app-dir/app-css/app/hmr/import/actual-styles.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
body {
background-color: red;
}
1 change: 1 addition & 0 deletions test/e2e/app-dir/app-css/app/hmr/import/page.css
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import './actual-styles.css';
5 changes: 5 additions & 0 deletions test/e2e/app-dir/app-css/app/hmr/import/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import './page.css'

export default function Page() {
return <div>hello!</div>
}
34 changes: 34 additions & 0 deletions test/e2e/app-dir/app-css/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,40 @@ createNextDescribe(
}
})

it('should reload @import styles during HMR', async () => {
const filePath = 'app/hmr/import/actual-styles.css'
const origContent = await next.readFile(filePath)

// background should be red
const browser = await next.browser('/hmr/import')
expect(
await browser.eval(
`window.getComputedStyle(document.querySelector('body')).backgroundColor`
)
).toBe('rgb(255, 0, 0)')

try {
await next.patchFile(
filePath,
origContent.replace(
'background-color: red;',
'background-color: blue;'
)
)

// Wait for HMR to trigger
await check(
() =>
browser.eval(
`window.getComputedStyle(document.querySelector('body')).backgroundColor`
),
'rgb(0, 0, 255)'
)
} finally {
await next.patchFile(filePath, origContent)
}
})

describe('multiple entries', () => {
it('should only inject the same style once if used by different layers', async () => {
const browser = await next.browser('/css/css-duplicate-2/client')
Expand Down

0 comments on commit 881d202

Please sign in to comment.