From d133944712f88329478b32c2e3549e484b6a837d Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Thu, 11 Jan 2024 15:41:12 +0900 Subject: [PATCH 1/9] (debug) debug ci --- .github/workflows/shared-test-integration.yml | 4 +++- integration/helpers/vite.ts | 2 ++ integration/playwright.config.ts | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/shared-test-integration.yml b/.github/workflows/shared-test-integration.yml index 97ec135aee9..6aa206ead96 100644 --- a/.github/workflows/shared-test-integration.yml +++ b/.github/workflows/shared-test-integration.yml @@ -56,4 +56,6 @@ jobs: run: npx playwright install --with-deps ${{ matrix.browser }} - name: 👀 Run Integration Tests ${{ matrix.browser }} - run: "yarn test:integration --project=${{ matrix.browser }}" + run: yarn test:integration --project=${{ matrix.browser }} --workers=1 integration/vite-hmr-hdr-test.ts -g dev + env: + REPEAT_EACH: 20 diff --git a/integration/helpers/vite.ts b/integration/helpers/vite.ts index bba3b627b33..720230ecccd 100644 --- a/integration/helpers/vite.ts +++ b/integration/helpers/vite.ts @@ -181,6 +181,8 @@ function node(args: string[], options: { cwd: string }) { env: process.env, stdio: "pipe", }); + proc.stdout?.on("data", data => process.stdout.write(data)) + proc.stderr?.on("data", data => process.stderr.write(data)) return proc; } diff --git a/integration/playwright.config.ts b/integration/playwright.config.ts index 62c3e39cfd3..2dc6d037797 100644 --- a/integration/playwright.config.ts +++ b/integration/playwright.config.ts @@ -13,6 +13,7 @@ const config: PlaywrightTestConfig = { }, forbidOnly: !!process.env.CI, retries: process.env.CI ? 3 : 0, + repeatEach: process.env.REPEAT_EACH ? Number(process.env.REPEAT_EACH) : undefined, reporter: process.env.CI ? "dot" : [["html", { open: "never" }]], use: { actionTimeout: 0 }, From 10196df93f597427628d3c2bd1af2ae7989d881a Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Thu, 11 Jan 2024 15:43:14 +0900 Subject: [PATCH 2/9] (debug) debug handleHotUpdate --- packages/remix-dev/vite/plugin.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts index a78c772a764..e214925f8cf 100644 --- a/packages/remix-dev/vite/plugin.ts +++ b/packages/remix-dev/vite/plugin.ts @@ -327,6 +327,10 @@ const getRouteModuleExports = async ( let [, exports] = esModuleLexer(transformed.code); let exportNames = exports.map((e) => e.n); + if (exportNames.length === 0) { + console.log("[getRouteMetadata:empty]", { routeFile, code }); + } + return exportNames; }; @@ -1383,6 +1387,7 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { ] as const ).some((key) => oldRouteMetadata[key] !== newRouteMetadata[key]) ) { + console.log("[handleHotUpdate:invalidateVirtualModules]", { file, oldRouteMetadata, newRouteMetadata }); invalidateVirtualModules(server); } } From c79aacddc024df567310f6cd13dc2eddb8a64777 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Thu, 11 Jan 2024 16:13:55 +0900 Subject: [PATCH 3/9] fix: use `HmrContext.read` instead of direct `fs.readFile` --- packages/remix-dev/vite/plugin.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts index e214925f8cf..5525d6bd78f 100644 --- a/packages/remix-dev/vite/plugin.ts +++ b/packages/remix-dev/vite/plugin.ts @@ -294,7 +294,8 @@ const getRouteManifestModuleExports = async ( const getRouteModuleExports = async ( viteChildCompiler: Vite.ViteDevServer | null, pluginConfig: ResolvedRemixVitePluginConfig, - routeFile: string + routeFile: string, + read?: () => string | Promise ): Promise => { if (!viteChildCompiler) { throw new Error("Vite child compiler not found"); @@ -318,7 +319,7 @@ const getRouteModuleExports = async ( let [id, code] = await Promise.all([ resolveId(), - fse.readFile(routePath, "utf-8"), + read ? read() : fse.readFile(routePath, "utf-8"), // pluginContainer.transform(...) fails if we don't do this first: moduleGraph.ensureEntryFromUrl(url, ssr), ]); @@ -1351,7 +1352,7 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { }, { name: "remix-hmr-updates", - async handleHotUpdate({ server, file, modules }) { + async handleHotUpdate({ server, file, modules, read }) { let pluginConfig = await resolvePluginConfig(); // Update the config cache any time there is a file change cachedPluginConfig = pluginConfig; @@ -1370,7 +1371,8 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { let newRouteMetadata = await getRouteMetadata( pluginConfig, viteChildCompiler, - route + route, + read ); hmrEventData.route = newRouteMetadata; @@ -1484,12 +1486,14 @@ function getRoute( async function getRouteMetadata( pluginConfig: ResolvedRemixVitePluginConfig, viteChildCompiler: Vite.ViteDevServer | null, - route: ConfigRoute + route: ConfigRoute, + read: () => string | Promise ) { let sourceExports = await getRouteModuleExports( viteChildCompiler, pluginConfig, - route.file + route.file, + read, ); let info = { From ac2b5b9aa90380e75c3647e3c42d0c1ab374ea1e Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Thu, 11 Jan 2024 16:34:22 +0900 Subject: [PATCH 4/9] Revert "(debug) debug handleHotUpdate" This reverts commit 10196df93f597427628d3c2bd1af2ae7989d881a. --- packages/remix-dev/vite/plugin.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts index 5525d6bd78f..2a8a867019f 100644 --- a/packages/remix-dev/vite/plugin.ts +++ b/packages/remix-dev/vite/plugin.ts @@ -328,10 +328,6 @@ const getRouteModuleExports = async ( let [, exports] = esModuleLexer(transformed.code); let exportNames = exports.map((e) => e.n); - if (exportNames.length === 0) { - console.log("[getRouteMetadata:empty]", { routeFile, code }); - } - return exportNames; }; @@ -1389,7 +1385,6 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { ] as const ).some((key) => oldRouteMetadata[key] !== newRouteMetadata[key]) ) { - console.log("[handleHotUpdate:invalidateVirtualModules]", { file, oldRouteMetadata, newRouteMetadata }); invalidateVirtualModules(server); } } From c676401e2dbac9392260e6de1ae228674755c877 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Thu, 11 Jan 2024 16:34:30 +0900 Subject: [PATCH 5/9] Revert "(debug) debug ci" This reverts commit d133944712f88329478b32c2e3549e484b6a837d. --- .github/workflows/shared-test-integration.yml | 4 +--- integration/helpers/vite.ts | 2 -- integration/playwright.config.ts | 1 - 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/.github/workflows/shared-test-integration.yml b/.github/workflows/shared-test-integration.yml index 6aa206ead96..97ec135aee9 100644 --- a/.github/workflows/shared-test-integration.yml +++ b/.github/workflows/shared-test-integration.yml @@ -56,6 +56,4 @@ jobs: run: npx playwright install --with-deps ${{ matrix.browser }} - name: 👀 Run Integration Tests ${{ matrix.browser }} - run: yarn test:integration --project=${{ matrix.browser }} --workers=1 integration/vite-hmr-hdr-test.ts -g dev - env: - REPEAT_EACH: 20 + run: "yarn test:integration --project=${{ matrix.browser }}" diff --git a/integration/helpers/vite.ts b/integration/helpers/vite.ts index 720230ecccd..bba3b627b33 100644 --- a/integration/helpers/vite.ts +++ b/integration/helpers/vite.ts @@ -181,8 +181,6 @@ function node(args: string[], options: { cwd: string }) { env: process.env, stdio: "pipe", }); - proc.stdout?.on("data", data => process.stdout.write(data)) - proc.stderr?.on("data", data => process.stderr.write(data)) return proc; } diff --git a/integration/playwright.config.ts b/integration/playwright.config.ts index 2dc6d037797..62c3e39cfd3 100644 --- a/integration/playwright.config.ts +++ b/integration/playwright.config.ts @@ -13,7 +13,6 @@ const config: PlaywrightTestConfig = { }, forbidOnly: !!process.env.CI, retries: process.env.CI ? 3 : 0, - repeatEach: process.env.REPEAT_EACH ? Number(process.env.REPEAT_EACH) : undefined, reporter: process.env.CI ? "dot" : [["html", { open: "never" }]], use: { actionTimeout: 0 }, From 7158156314f59ce214bdc61e3579403d29301eb1 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Thu, 11 Jan 2024 17:28:58 +0900 Subject: [PATCH 6/9] chore: stuck github action From c864b28691dba6b064e37a0d379f0c8f823fda23 Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Fri, 12 Jan 2024 08:16:41 +1100 Subject: [PATCH 7/9] refactor --- packages/remix-dev/vite/plugin.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts index 2a8a867019f..00f47d9199c 100644 --- a/packages/remix-dev/vite/plugin.ts +++ b/packages/remix-dev/vite/plugin.ts @@ -295,7 +295,8 @@ const getRouteModuleExports = async ( viteChildCompiler: Vite.ViteDevServer | null, pluginConfig: ResolvedRemixVitePluginConfig, routeFile: string, - read?: () => string | Promise + readRouteFile: () => string | Promise = () => + fse.readFile(routeFile, "utf-8") ): Promise => { if (!viteChildCompiler) { throw new Error("Vite child compiler not found"); @@ -319,7 +320,7 @@ const getRouteModuleExports = async ( let [id, code] = await Promise.all([ resolveId(), - read ? read() : fse.readFile(routePath, "utf-8"), + readRouteFile(), // pluginContainer.transform(...) fails if we don't do this first: moduleGraph.ensureEntryFromUrl(url, ssr), ]); @@ -1482,13 +1483,13 @@ async function getRouteMetadata( pluginConfig: ResolvedRemixVitePluginConfig, viteChildCompiler: Vite.ViteDevServer | null, route: ConfigRoute, - read: () => string | Promise + readRouteFile?: () => string | Promise ) { let sourceExports = await getRouteModuleExports( viteChildCompiler, pluginConfig, route.file, - read, + readRouteFile ); let info = { From b3f7b566406e172057c06d6525981c19b22eeaab Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Fri, 12 Jan 2024 08:18:42 +1100 Subject: [PATCH 8/9] add changeset --- .changeset/olive-vans-explain.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/olive-vans-explain.md diff --git a/.changeset/olive-vans-explain.md b/.changeset/olive-vans-explain.md new file mode 100644 index 00000000000..23eb0d87a25 --- /dev/null +++ b/.changeset/olive-vans-explain.md @@ -0,0 +1,5 @@ +--- +"@remix-run/dev": patch +--- + +Vite: Fix HMR race condition when reading changed file contents From 61939b3232a0318c4360f1e28aa550f0939cc228 Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Fri, 12 Jan 2024 09:25:23 +1100 Subject: [PATCH 9/9] fix --- packages/remix-dev/vite/plugin.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts index 628f727ecea..55058dc774b 100644 --- a/packages/remix-dev/vite/plugin.ts +++ b/packages/remix-dev/vite/plugin.ts @@ -295,8 +295,7 @@ const getRouteModuleExports = async ( viteChildCompiler: Vite.ViteDevServer | null, pluginConfig: ResolvedRemixVitePluginConfig, routeFile: string, - readRouteFile: () => string | Promise = () => - fse.readFile(routeFile, "utf-8") + readRouteFile?: () => string | Promise ): Promise => { if (!viteChildCompiler) { throw new Error("Vite child compiler not found"); @@ -320,7 +319,7 @@ const getRouteModuleExports = async ( let [id, code] = await Promise.all([ resolveId(), - readRouteFile(), + readRouteFile?.() ?? fse.readFile(routePath, "utf-8"), // pluginContainer.transform(...) fails if we don't do this first: moduleGraph.ensureEntryFromUrl(url, ssr), ]);