From 946ef954a97ff88b11ef55b87988834f5efd8acf Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Fri, 3 Nov 2023 22:14:33 +0900 Subject: [PATCH 1/8] fix(vite): use `ssrEmitAssets` to support server only asset --- packages/remix-dev/vite/plugin.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts index c7e04f00527..2d859085601 100644 --- a/packages/remix-dev/vite/plugin.ts +++ b/packages/remix-dev/vite/plugin.ts @@ -462,6 +462,8 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { }, } : { + // unstable option? https://github.com/vitejs/vite/discussions/13808 + ssrEmitAssets: true, outDir: path.dirname(pluginConfig.serverBuildPath), rollupOptions: { ...viteUserConfig.build?.rollupOptions, @@ -479,6 +481,28 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { }), }; }, + // after ssr asset generation, move them to client assets directory + // https://rollupjs.org/plugin-development/#writebundle + // we could intercept during `generateBundle` and write files directly + // but that would lose file name/size logging by rollup + // https://rollupjs.org/plugin-development/#generatebundle + writeBundle: { + async handler(options, bundle) { + if (!ssrBuildContext.isSsrBuild) { + return; + } + let pluginConfig = await resolvePluginConfig(); + for (const entry of Object.values(bundle)) { + if (entry.type === "asset") { + // TODO: when is "dir" undefined? + await fs.rename( + path.join(options.dir!, entry.fileName), + path.join(pluginConfig.assetsBuildDirectory, entry.fileName) + ); + } + } + }, + }, async configResolved(viteConfig) { await initEsModuleLexer; From fadbc25927aa864e069aaa83138ab20f1ccdd15f Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Fri, 3 Nov 2023 22:38:10 +0900 Subject: [PATCH 2/8] test: add test --- integration/vite-build-test.ts | 42 ++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/integration/vite-build-test.ts b/integration/vite-build-test.ts index 45cfdf5a3f8..02da3fd7ac2 100644 --- a/integration/vite-build-test.ts +++ b/integration/vite-build-test.ts @@ -29,6 +29,10 @@ test.describe("Vite build", () => { export default defineConfig({ plugins: [remix()], + build: { + // force emitting asset files instead of inline as data-url + assetsInlineLimit: 0, + } }); `, "app/root.tsx": js` @@ -99,6 +103,29 @@ test.describe("Vite build", () => { export const serverOnly1 = "SERVER_ONLY_1" export const serverOnly2 = "SERVER_ONLY_2" `, + + "app/routes/ssr-assets.tsx": js` + import url1 from "../assets/test1.txt?url"; + import url2 from "../assets/test2.txt?url"; + import { useLoaderData } from "@remix-run/react" + + export const loader: LoaderFunction = () => { + return { url2 }; + }; + + export default function SsrAssetRoute() { + const loaderData = useLoaderData(); + return ( +
+ url1 + url2 +
+ ); + } + `, + + "app/assets/test1.txt": "test1", + "app/assets/test2.txt": "test2", }, }); @@ -146,4 +173,19 @@ test.describe("Vite build", () => { "Mounted" ); }); + + test("emit ssr assets", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/ssr-assets"); + + // verify asset files are emitted and served correctly + await page.getByRole('link', { name: 'url1' }).click(); + await page.waitForURL("**/build/assets/test1-1b4f0e98.txt"); + await page.getByText('test1').click(); + await page.goBack(); + + await page.getByRole('link', { name: 'url2' }).click(); + await page.waitForURL("**/build/assets/test2-60303ae2.txt"); + await page.getByText('test2').click(); + }); }); From 1a225c0a9dc1498646431363e4305eee6eaa6da6 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Sun, 5 Nov 2023 12:06:14 +0900 Subject: [PATCH 3/8] chore: changeset --- .changeset/tricky-frogs-film.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/tricky-frogs-film.md diff --git a/.changeset/tricky-frogs-film.md b/.changeset/tricky-frogs-film.md new file mode 100644 index 00000000000..d9a3ad12a22 --- /dev/null +++ b/.changeset/tricky-frogs-film.md @@ -0,0 +1,5 @@ +--- +"@remix-run/dev": patch +--- + +fix(vite): emit asset files references only by server-only file From 86551828bae4b1fe7c48d3a8017b84c982e02c88 Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Wed, 15 Nov 2023 15:34:03 +1100 Subject: [PATCH 4/8] only move ssr-only assets, clean other ssr assets --- integration/vite-build-test.ts | 30 +++--- packages/remix-dev/vite/plugin.ts | 148 ++++++++++++++++++++++-------- 2 files changed, 123 insertions(+), 55 deletions(-) diff --git a/integration/vite-build-test.ts b/integration/vite-build-test.ts index 2423277cee6..6d5d8461aa0 100644 --- a/integration/vite-build-test.ts +++ b/integration/vite-build-test.ts @@ -258,21 +258,6 @@ test.describe("Vite build", () => { ); }); - test("emit ssr assets", async ({ page }) => { - let app = new PlaywrightFixture(appFixture, page); - await app.goto("/ssr-assets"); - - // verify asset files are emitted and served correctly - await page.getByRole("link", { name: "url1" }).click(); - await page.waitForURL("**/build/assets/test1-*.txt"); - await page.getByText("test1").click(); - await page.goBack(); - - await page.getByRole("link", { name: "url2" }).click(); - await page.waitForURL("**/build/assets/test2-*.txt"); - await page.getByText("test2").click(); - }); - test("server renders matching MDX routes", async ({ page }) => { let res = await fixture.requestDocument("/mdx"); expect(res.status).toBe(200); @@ -294,6 +279,21 @@ test.describe("Vite build", () => { expect(pageErrors).toEqual([]); }); + test("emits SSR assets to the client assets directory", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/ssr-assets"); + + // verify asset files are emitted and served correctly + await page.getByRole("link", { name: "url1" }).click(); + await page.waitForURL("**/build/assets/test1-*.txt"); + await page.getByText("test1").click(); + await page.goBack(); + + await page.getByRole("link", { name: "url2" }).click(); + await page.waitForURL("**/build/assets/test2-*.txt"); + await page.getByText("test2").click(); + }); + test("supports code-split css", async ({ page }) => { let pageErrors: unknown[] = []; page.on("pageerror", (error) => pageErrors.push(error)); diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts index 72e1cce16c2..2bd507b93e7 100644 --- a/packages/remix-dev/vite/plugin.ts +++ b/packages/remix-dev/vite/plugin.ts @@ -3,7 +3,7 @@ import type * as Vite from "vite"; import { type BinaryLike, createHash } from "node:crypto"; import * as path from "node:path"; -import * as fs from "node:fs/promises"; +import * as fse from "fs-extra"; import babel from "@babel/core"; import { type ServerBuild } from "@remix-run/server-runtime"; import { @@ -182,8 +182,8 @@ function dedupe(array: T[]): T[] { } const writeFileSafe = async (file: string, contents: string): Promise => { - await fs.mkdir(path.dirname(file), { recursive: true }); - await fs.writeFile(file, contents); + await fse.ensureDir(path.dirname(file)); + await fse.writeFile(file, contents); }; const getRouteModuleExports = async ( @@ -213,7 +213,7 @@ const getRouteModuleExports = async ( let [id, code] = await Promise.all([ resolveId(), - fs.readFile(routePath, "utf-8"), + fse.readFile(routePath, "utf-8"), // pluginContainer.transform(...) fails if we don't do this first: moduleGraph.ensureEntryFromUrl(url, ssr), ]); @@ -244,6 +244,8 @@ export type RemixVitePlugin = ( export const remixVitePlugin: RemixVitePlugin = (options = {}) => { let viteCommand: Vite.ResolvedConfig["command"]; let viteUserConfig: Vite.UserConfig; + let resolvedViteConfig: Vite.ResolvedConfig | undefined; + let isViteV4 = getViteMajorVersion() === 4; let cssModulesManifest: Record = {}; @@ -338,19 +340,23 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { };`; }; - let createBuildManifest = async (): Promise => { - let pluginConfig = await resolvePluginConfig(); - - let viteManifestPath = isViteV4 + let loadViteManifest = async (directory: string) => { + let manifestPath = isViteV4 ? "manifest.json" : path.join(".vite", "manifest.json"); + let manifestContents = await fse.readFile( + path.resolve(directory, manifestPath), + "utf-8" + ); + return JSON.parse(manifestContents) as Vite.Manifest; + }; - let viteManifest = JSON.parse( - await fs.readFile( - path.resolve(pluginConfig.assetsBuildDirectory, viteManifestPath), - "utf-8" - ) - ) as Vite.Manifest; + let createBuildManifest = async (): Promise => { + let pluginConfig = await resolvePluginConfig(); + + let viteManifest = await loadViteManifest( + pluginConfig.assetsBuildDirectory + ); let entry: Manifest["entry"] = resolveBuildAssetPaths( pluginConfig, @@ -529,8 +535,8 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { }, } : { - // unstable option? https://github.com/vitejs/vite/discussions/13808 - ssrEmitAssets: true, + ssrEmitAssets: true, // We move SSR-only assets to client assets and clean the rest + manifest: true, // We need the manifest to detect SSR-only assets outDir: path.dirname(pluginConfig.serverBuildPath), rollupOptions: { ...viteUserConfig.build?.rollupOptions, @@ -548,31 +554,11 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { }), }; }, - // after ssr asset generation, move them to client assets directory - // https://rollupjs.org/plugin-development/#writebundle - // we could intercept during `generateBundle` and write files directly - // but that would lose file name/size logging by rollup - // https://rollupjs.org/plugin-development/#generatebundle - writeBundle: { - async handler(options, bundle) { - if (!ssrBuildContext.isSsrBuild) { - return; - } - let pluginConfig = await resolvePluginConfig(); - for (const entry of Object.values(bundle)) { - if (entry.type === "asset") { - // TODO: when is "dir" undefined? - await fs.rename( - path.join(options.dir!, entry.fileName), - path.join(pluginConfig.assetsBuildDirectory, entry.fileName) - ); - } - } - }, - }, async configResolved(viteConfig) { await initEsModuleLexer; + resolvedViteConfig = viteConfig; + ssrBuildContext = viteConfig.build.ssr && viteCommand === "build" ? { isSsrBuild: true, getManifest: createBuildManifest } @@ -761,6 +747,88 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { } }; }, + writeBundle: { + // After the SSR build is finished, we inspect the Vite manifest for + // the SSR build and move all server assets to client assets directory + async handler() { + if (!ssrBuildContext.isSsrBuild) { + return; + } + + invariant( + cachedPluginConfig, + "Expected plugin config to be cached when writeBundle hook is called" + ); + + invariant( + resolvedViteConfig, + "Expected resolvedViteConfig to exist when writeBundle hook is called" + ); + + let { assetsBuildDirectory, serverBuildPath, rootDirectory } = + cachedPluginConfig; + let serverBuildDir = path.dirname(serverBuildPath); + + let ssrViteManifest = await loadViteManifest(serverBuildDir); + let clientViteManifest = await loadViteManifest(assetsBuildDirectory); + + let clientAssetPaths = new Set( + Object.values(clientViteManifest).flatMap( + (chunk) => chunk.assets ?? [] + ) + ); + + let ssrOnlyAssetPaths = new Set( + Object.values(ssrViteManifest) + .flatMap((chunk) => chunk.assets ?? []) + // Only move assets that aren't in the client build + .filter((asset) => !clientAssetPaths.has(asset)) + ); + + let movedAssetPaths = await Promise.all( + Array.from(ssrOnlyAssetPaths).map(async (ssrAssetPath) => { + let src = path.join(serverBuildDir, ssrAssetPath); + let dest = path.join(assetsBuildDirectory, ssrAssetPath); + await fse.move(src, dest); + return dest; + }) + ); + + let logger = vite.createLogger(); + + if (movedAssetPaths.length) { + logger.info( + [ + "", + `${colors.green("✓")} ${movedAssetPaths.length} asset${ + movedAssetPaths.length > 1 ? "s" : "" + } moved from Remix server build to client assets.`, + ...movedAssetPaths.map((movedAssetPath) => + colors.dim(path.relative(rootDirectory, movedAssetPath)) + ), + ].join("\n") + ); + } + + let ssrAssetsDir = path.join( + resolvedViteConfig.build.outDir, + resolvedViteConfig.build.assetsDir + ); + + if (fse.existsSync(ssrAssetsDir)) { + await fse.remove(ssrAssetsDir); + logger.info( + [ + "", + colors.dim( + `${colors.green("✓")} Cleaned Remix server assets directory.` + ), + colors.dim(path.relative(rootDirectory, ssrAssetsDir)), + ].join("\n") + ); + } + }, + }, async buildEnd() { await viteChildCompiler?.close(); }, @@ -921,8 +989,8 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { return [ "const exports = {}", - await fs.readFile(reactRefreshRuntimePath, "utf8"), - await fs.readFile( + await fse.readFile(reactRefreshRuntimePath, "utf8"), + await fse.readFile( require.resolve("./static/refresh-utils.cjs"), "utf8" ), From daf03a66b4aa09ad2a917305aab4a097b7bbcdaf Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Wed, 15 Nov 2023 15:41:37 +0900 Subject: [PATCH 5/8] chore: typo in .changeset/tricky-frogs-film.md --- .changeset/tricky-frogs-film.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/tricky-frogs-film.md b/.changeset/tricky-frogs-film.md index d9a3ad12a22..1295151ea36 100644 --- a/.changeset/tricky-frogs-film.md +++ b/.changeset/tricky-frogs-film.md @@ -2,4 +2,4 @@ "@remix-run/dev": patch --- -fix(vite): emit asset files references only by server-only file +fix(vite): emit asset files referenced only by server-only file From c603995f719a975fcbf9c55564a68bd8a3d3ce30 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Wed, 15 Nov 2023 15:41:50 +0900 Subject: [PATCH 6/8] chore: typo in integration/vite-build-test.ts --- integration/vite-build-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/vite-build-test.ts b/integration/vite-build-test.ts index 6d5d8461aa0..5fd8a01fa4b 100644 --- a/integration/vite-build-test.ts +++ b/integration/vite-build-test.ts @@ -33,7 +33,7 @@ test.describe("Vite build", () => { export default defineConfig({ build: { - // force emitting asset files instead of inline as data-url + // force emitting asset files instead of inlined as data-url assetsInlineLimit: 0, }, plugins: [ From 5f9ef71d7c64ada162fe537ed1b236edaf8c0257 Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Thu, 16 Nov 2023 11:28:17 +1100 Subject: [PATCH 7/8] Apply suggestions from code review Co-authored-by: Hiroshi Ogawa Co-authored-by: Pedro Cattori --- .changeset/tricky-frogs-film.md | 2 +- packages/remix-dev/vite/plugin.ts | 11 +---------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/.changeset/tricky-frogs-film.md b/.changeset/tricky-frogs-film.md index 1295151ea36..8a67df991fd 100644 --- a/.changeset/tricky-frogs-film.md +++ b/.changeset/tricky-frogs-film.md @@ -2,4 +2,4 @@ "@remix-run/dev": patch --- -fix(vite): emit asset files referenced only by server-only file +Emit assets that were only referenced in the server build into the client assets directory in Vite build diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts index 2bd507b93e7..9c146594035 100644 --- a/packages/remix-dev/vite/plugin.ts +++ b/packages/remix-dev/vite/plugin.ts @@ -794,7 +794,7 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { }) ); - let logger = vite.createLogger(); + let logger = resolvedViteConfig.logger; if (movedAssetPaths.length) { logger.info( @@ -817,15 +817,6 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { if (fse.existsSync(ssrAssetsDir)) { await fse.remove(ssrAssetsDir); - logger.info( - [ - "", - colors.dim( - `${colors.green("✓")} Cleaned Remix server assets directory.` - ), - colors.dim(path.relative(rootDirectory, ssrAssetsDir)), - ].join("\n") - ); } }, }, From d85fd10d63912fc9d992568b5a0f3b001df1b5ee Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Thu, 16 Nov 2023 11:31:56 +1100 Subject: [PATCH 8/8] add trailing line break to terminal output --- packages/remix-dev/vite/plugin.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts index 9c146594035..60e4fb4ec0b 100644 --- a/packages/remix-dev/vite/plugin.ts +++ b/packages/remix-dev/vite/plugin.ts @@ -806,6 +806,7 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { ...movedAssetPaths.map((movedAssetPath) => colors.dim(path.relative(rootDirectory, movedAssetPath)) ), + "", ].join("\n") ); }