From 9103c6b3baa4854fa50116713fa35073271991d9 Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Wed, 24 Apr 2024 14:39:30 +1000 Subject: [PATCH 1/4] Only add RR packages to `ssr.external` in RR repo (#11489) --- packages/remix-dev/vite/plugin.ts | 50 ++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts index 0aa7f27be3..c9cef54c7e 100644 --- a/packages/remix-dev/vite/plugin.ts +++ b/packages/remix-dev/vite/plugin.ts @@ -1048,24 +1048,26 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = (_config) => { : "custom", ssr: { - external: [ - // This is only necessary for development within this repo - // because these packages are symlinked and Vite treats them as - // internal source code. For consumers this is a no-op. - "react-router", - "react-router-dom", - "@react-router/architect", - "@react-router/cloudflare-pages", - "@react-router/cloudflare-workers", - "@react-router/cloudflare", - "@react-router/deno", - "@react-router/dev", - "@react-router/express", - "@react-router/netlify", - "@react-router/node", - "@react-router/serve", - "@react-router/server-runtime", - ], + external: isInReactRouterMonorepo() + ? [ + // This is only needed within this repo because these packages + // are linked to a directory outside of node_modules so Vite + // treats them as internal code by default. + "react-router", + "react-router-dom", + "@react-router/architect", + "@react-router/cloudflare-pages", + "@react-router/cloudflare-workers", + "@react-router/cloudflare", + "@react-router/deno", + "@react-router/dev", + "@react-router/express", + "@react-router/netlify", + "@react-router/node", + "@react-router/serve", + "@react-router/server-runtime", + ] + : undefined, }, optimizeDeps: { include: [ @@ -1779,6 +1781,18 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = (_config) => { ]; }; +function isInReactRouterMonorepo() { + // We use '@react-router/server-runtime' for this check since it's a + // dependency of this package and guaranteed to be in node_modules + let serverRuntimePath = path.dirname( + require.resolve("@react-router/server-runtime/package.json") + ); + let serverRuntimeParentDir = path.basename( + path.resolve(serverRuntimePath, "..") + ); + return serverRuntimeParentDir === "packages"; +} + function isEqualJson(v1: unknown, v2: unknown) { return JSON.stringify(v1) === JSON.stringify(v2); } From f322d47fe6b3330e2b52b2dd052237653e125dd6 Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Wed, 24 Apr 2024 14:39:49 +1000 Subject: [PATCH 2/4] Remove `@react-router/node` from `optimizeDeps.include` (#11490) --- packages/remix-dev/vite/plugin.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts index c9cef54c7e..2a9ec14c7a 100644 --- a/packages/remix-dev/vite/plugin.ts +++ b/packages/remix-dev/vite/plugin.ts @@ -1083,13 +1083,6 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = (_config) => { // Mismatching routers cause `Error: You must render this element inside a element`. "react-router", "react-router-dom", - - // For some reason, the `vite-dotenv` integration test consistently fails on webkit - // with `504 (Outdated Optimize Dep)` from Vite unless `@react-router/node` is included - // in `optimizeDeps.include`. 🤷 - // This could be caused by how we copy `node_modules/` into integration test fixtures, - // so maybe this will be unnecessary once we switch to pnpm - "@react-router/node", ], }, esbuild: { From 221a2c94a3854cdeca4b47fa1ae5925ed94fdca5 Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Fri, 26 Apr 2024 15:01:05 +1000 Subject: [PATCH 3/4] Fix `dest already exists error` during Vite build (#11501) --- packages/remix-dev/vite/plugin.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts index 2a9ec14c7a..72ee1285d5 100644 --- a/packages/remix-dev/vite/plugin.ts +++ b/packages/remix-dev/vite/plugin.ts @@ -821,6 +821,11 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = (_config) => { return JSON.parse(manifestContents) as Vite.Manifest; }; + let getViteManifestFilePaths = (viteManifest: Vite.Manifest): Set => { + let filePaths = Object.values(viteManifest).map((chunk) => chunk.file); + return new Set(filePaths); + }; + let getViteManifestAssetPaths = ( viteManifest: Vite.Manifest ): Set => { @@ -1405,7 +1410,7 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = (_config) => { let ssrViteManifest = await loadViteManifest(serverBuildDirectory); let clientViteManifest = await loadViteManifest(clientBuildDirectory); - let clientAssetPaths = getViteManifestAssetPaths(clientViteManifest); + let clientFilePaths = getViteManifestFilePaths(clientViteManifest); let ssrAssetPaths = getViteManifestAssetPaths(ssrViteManifest); // We only move assets that aren't in the client build, otherwise we @@ -1417,7 +1422,7 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = (_config) => { let movedAssetPaths: string[] = []; for (let ssrAssetPath of ssrAssetPaths) { let src = path.join(serverBuildDirectory, ssrAssetPath); - if (!clientAssetPaths.has(ssrAssetPath)) { + if (!clientFilePaths.has(ssrAssetPath)) { let dest = path.join(clientBuildDirectory, ssrAssetPath); await fse.move(src, dest); movedAssetPaths.push(dest); From b98c54936437a38f17732113c455a43c7546424f Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Wed, 1 May 2024 04:49:24 +1000 Subject: [PATCH 4/4] Skip `vite-route-added-test` in Safari (#11511) --- integration/vite-route-added-test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/integration/vite-route-added-test.ts b/integration/vite-route-added-test.ts index 0cd0ce7b6a..b93c50c48c 100644 --- a/integration/vite-route-added-test.ts +++ b/integration/vite-route-added-test.ts @@ -38,7 +38,12 @@ test.describe(async () => { }); test.afterAll(() => stop()); - test("Vite / dev / route added", async ({ page }) => { + test("Vite / dev / route added", async ({ page, browserName }) => { + test.skip( + browserName === "webkit", + "Safari caches too aggressively, browser manifest is cached with old routes" + ); + let pageErrors: Error[] = []; page.on("pageerror", (error) => pageErrors.push(error));