From 881d202962b4226330b6fe4986d0cc943e4f40e6 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Tue, 9 May 2023 17:48:19 +0200 Subject: [PATCH] Fix server CSS imports and HMR not working properly in specific conditions (#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 --- packages/next/src/server/dev/hot-reloader.ts | 8 ++++- .../app-css/app/hmr/import/actual-styles.css | 3 ++ .../app-dir/app-css/app/hmr/import/page.css | 1 + .../app-dir/app-css/app/hmr/import/page.js | 5 +++ test/e2e/app-dir/app-css/index.test.ts | 34 +++++++++++++++++++ 5 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 test/e2e/app-dir/app-css/app/hmr/import/actual-styles.css create mode 100644 test/e2e/app-dir/app-css/app/hmr/import/page.css create mode 100644 test/e2e/app-dir/app-css/app/hmr/import/page.js diff --git a/packages/next/src/server/dev/hot-reloader.ts b/packages/next/src/server/dev/hot-reloader.ts index 0cdd2be15041d..d0cd8b1f6302e 100644 --- a/packages/next/src/server/dev/hot-reloader.ts +++ b/packages/next/src/server/dev/hot-reloader.ts @@ -885,6 +885,10 @@ export default class HotReloader { const prevEdgeServerPageHashes = new Map() const prevCSSImportModuleHashes = new Map() + const pageExtensionRegex = new RegExp( + `\\.(?:${this.config.pageExtensions.join('|')})$` + ) + const trackPageChanges = ( pageHashMap: Map, @@ -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 diff --git a/test/e2e/app-dir/app-css/app/hmr/import/actual-styles.css b/test/e2e/app-dir/app-css/app/hmr/import/actual-styles.css new file mode 100644 index 0000000000000..aa1634c255034 --- /dev/null +++ b/test/e2e/app-dir/app-css/app/hmr/import/actual-styles.css @@ -0,0 +1,3 @@ +body { + background-color: red; +} diff --git a/test/e2e/app-dir/app-css/app/hmr/import/page.css b/test/e2e/app-dir/app-css/app/hmr/import/page.css new file mode 100644 index 0000000000000..b07652975060e --- /dev/null +++ b/test/e2e/app-dir/app-css/app/hmr/import/page.css @@ -0,0 +1 @@ +@import './actual-styles.css'; diff --git a/test/e2e/app-dir/app-css/app/hmr/import/page.js b/test/e2e/app-dir/app-css/app/hmr/import/page.js new file mode 100644 index 0000000000000..56b7b3371899a --- /dev/null +++ b/test/e2e/app-dir/app-css/app/hmr/import/page.js @@ -0,0 +1,5 @@ +import './page.css' + +export default function Page() { + return
hello!
+} diff --git a/test/e2e/app-dir/app-css/index.test.ts b/test/e2e/app-dir/app-css/index.test.ts index 7e19cf7c7013b..bebbd67b7d5e5 100644 --- a/test/e2e/app-dir/app-css/index.test.ts +++ b/test/e2e/app-dir/app-css/index.test.ts @@ -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')