Skip to content

Commit

Permalink
perf(remix-dev/vite): use fs watcher to invalidate manifest instead o…
Browse files Browse the repository at this point in the history
…f request time (#8164)

Co-authored-by: Mark Dalgleish <mark.john.dalgleish@gmail.com>
  • Loading branch information
hi-ogawa and markdalgleish committed Jan 12, 2024
1 parent 8c16935 commit 3218847
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 47 deletions.
71 changes: 71 additions & 0 deletions integration/vite-route-added-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import fs from "node:fs/promises";
import path from "node:path";
import { test, expect } from "@playwright/test";
import getPort from "get-port";

import { createProject, viteDev, VITE_CONFIG } from "./helpers/vite.js";

const files = {
"app/routes/_index.tsx": String.raw`
import { useState, useEffect } from "react";
import { Link } from "@remix-run/react";
export default function IndexRoute() {
const [mounted, setMounted] = useState(false);
useEffect(() => {
setMounted(true);
}, []);
return (
<p data-mounted>Mounted: {mounted ? "yes" : "no"}</p>
);
}
`,
};

test.describe(async () => {
let port: number;
let cwd: string;
let stop: () => void;

test.beforeAll(async () => {
port = await getPort();
cwd = await createProject({
"vite.config.js": await VITE_CONFIG({ port }),
...files,
});
stop = await viteDev({ cwd, port });
});
test.afterAll(async () => await stop());

test("Vite / dev / route added", async ({ page }) => {
let pageErrors: Error[] = [];
page.on("pageerror", (error) => pageErrors.push(error));

// wait for hydration to make sure initial virtual modules are loaded
await page.goto(`http://localhost:${port}/`, { waitUntil: "networkidle" });
await expect(page.locator("[data-mounted]")).toHaveText("Mounted: yes");

// add new route file
await fs.writeFile(
path.join(cwd, "app/routes/new.tsx"),
String.raw`
export default function Route() {
return (
<div id="new">new route</div>
);
}
`,
"utf-8"
);

// client is not notified of new route addition (https://github.com/remix-run/remix/issues/7894)
// however server can handle new route
await expect
.poll(async () => {
await page.goto(`http://localhost:${port}/new`);
return page.getByText("new route").isVisible();
})
.toBe(true);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ test.describe(async () => {
});
test.afterAll(() => stop());

test("Vite / dev / invalidate manifest on route exports change", async ({
test("Vite / dev / route exports modified offscreen", async ({
page,
context,
browserName,
Expand Down
79 changes: 33 additions & 46 deletions packages/remix-dev/vite/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,10 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
| { isSsrBuild: true; getManifest: () => Promise<Manifest> };

let viteChildCompiler: Vite.ViteDevServer | null = null;
let cachedPluginConfig: ResolvedRemixVitePluginConfig | undefined;

// This is initialized during `config` hook, so most of the code can assume this is defined without null check.
// During dev, this is updated on config file change or route file addition/removal.
let pluginConfig: ResolvedRemixVitePluginConfig;

let resolveServerBuildConfig = (): ServerBuildConfig | null => {
if (
Expand Down Expand Up @@ -448,8 +451,6 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
};

let getServerEntry = async () => {
let pluginConfig = await resolvePluginConfig();

return `
import * as entryServer from ${JSON.stringify(
resolveFileUrl(pluginConfig, pluginConfig.entryServerFilePath)
Expand Down Expand Up @@ -502,8 +503,6 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
};

let createBuildManifest = async (): Promise<Manifest> => {
let pluginConfig = await resolvePluginConfig();

let viteManifest = await loadViteManifest(
pluginConfig.assetsBuildDirectory
);
Expand Down Expand Up @@ -569,7 +568,6 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
};

let getDevManifest = async (): Promise<Manifest> => {
let pluginConfig = await resolvePluginConfig();
let routes: Manifest["routes"] = {};

let routeManifestExports = await getRouteManifestModuleExports(
Expand Down Expand Up @@ -625,8 +623,7 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
viteUserConfig = _viteUserConfig;
viteCommand = viteConfigEnv.command;

let pluginConfig = await resolvePluginConfig();
cachedPluginConfig = pluginConfig;
pluginConfig = await resolvePluginConfig();

Object.assign(
process.env,
Expand Down Expand Up @@ -816,11 +813,10 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
}

if (id.endsWith(CLIENT_ROUTE_QUERY_STRING)) {
invariant(cachedPluginConfig);
let routeModuleId = id.replace(CLIENT_ROUTE_QUERY_STRING, "");
let sourceExports = await getRouteModuleExports(
viteChildCompiler,
cachedPluginConfig,
pluginConfig,
routeModuleId
);

Expand Down Expand Up @@ -865,10 +861,9 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
// Give the request handler access to the critical CSS in dev to avoid a
// flash of unstyled content since Vite injects CSS file contents via JS
getCriticalCss: async (build, url) => {
invariant(cachedPluginConfig);
return getStylesForUrl(
viteDevServer,
cachedPluginConfig,
pluginConfig,
cssModulesManifest,
build,
url
Expand All @@ -883,32 +878,32 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
},
});

// We cache the pluginConfig here to make sure we're only invalidating virtual modules when necessary.
// This requires a separate cache from `cachedPluginConfig`, which is updated by remix-hmr-updates. If
// we shared the cache, it would already be refreshed by remix-hmr-updates at this point, and we'd
// have no way of comparing against the cache to know if the virtual modules need to be invalidated.
let previousPluginConfig: ResolvedRemixVitePluginConfig | undefined;
// Invalidate virtual modules and update cached plugin config via file watcher
viteDevServer.watcher.on("all", async (eventName, filepath) => {
let { normalizePath } = importViteEsmSync();

return () => {
viteDevServer.middlewares.use(async (_req, _res, next) => {
try {
let pluginConfig = await resolvePluginConfig();
let appFileAddedOrRemoved =
(eventName === "add" || eventName === "unlink") &&
normalizePath(filepath).startsWith(
normalizePath(pluginConfig.appDirectory)
);

if (
JSON.stringify(pluginConfig) !==
JSON.stringify(previousPluginConfig)
) {
previousPluginConfig = pluginConfig;
invariant(viteConfig?.configFile);
let viteConfigChanged =
eventName === "change" &&
normalizePath(filepath) === normalizePath(viteConfig.configFile);

invalidateVirtualModules(viteDevServer);
}
if (appFileAddedOrRemoved || viteConfigChanged) {
let lastPluginConfig = pluginConfig;
pluginConfig = await resolvePluginConfig();

next();
} catch (error) {
next(error);
if (!isEqualJson(lastPluginConfig, pluginConfig)) {
invalidateVirtualModules(viteDevServer);
}
});
}
});

return () => {
// Let user servers handle SSR requests in middleware mode,
// otherwise the Vite plugin will handle the request
if (!viteDevServer.config.server.middlewareMode) {
Expand Down Expand Up @@ -938,15 +933,14 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
return;
}

invariant(cachedPluginConfig);
invariant(viteConfig);

let {
assetsBuildDirectory,
serverBuildDirectory,
serverBuildFile,
rootDirectory,
} = cachedPluginConfig;
} = pluginConfig;

let ssrViteManifest = await loadViteManifest(serverBuildDirectory);
let clientViteManifest = await loadViteManifest(assetsBuildDirectory);
Expand Down Expand Up @@ -1007,7 +1001,7 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
);
}

if (cachedPluginConfig.isSpaMode) {
if (pluginConfig.isSpaMode) {
await handleSpaMode(
path.join(rootDirectory, serverBuildDirectory),
serverBuildFile,
Expand Down Expand Up @@ -1078,7 +1072,6 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
}

let vite = importViteEsmSync();
let pluginConfig = await resolvePluginConfig();
let importerShort = vite.normalizePath(
path.relative(pluginConfig.rootDirectory, importer)
);
Expand Down Expand Up @@ -1145,8 +1138,6 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
async transform(code, id, options) {
if (options?.ssr) return;

let pluginConfig = cachedPluginConfig || (await resolvePluginConfig());

let route = getRoute(pluginConfig, id);
if (!route) return;

Expand Down Expand Up @@ -1296,9 +1287,6 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
if (!useFastRefresh) return;

if (id.endsWith(CLIENT_ROUTE_QUERY_STRING)) {
let pluginConfig =
cachedPluginConfig || (await resolvePluginConfig());

return { code: addRefreshWrapper(pluginConfig, code, id) };
}

Expand All @@ -1317,8 +1305,6 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
code = result.code!;
let refreshContentRE = /\$Refresh(?:Reg|Sig)\$\(/;
if (refreshContentRE.test(code)) {
let pluginConfig =
cachedPluginConfig || (await resolvePluginConfig());
code = addRefreshWrapper(pluginConfig, code, id);
}
return { code, map: result.map };
Expand All @@ -1327,9 +1313,6 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
{
name: "remix-hmr-updates",
async handleHotUpdate({ server, file, modules, read }) {
let pluginConfig = await resolvePluginConfig();
// Update the config cache any time there is a file change
cachedPluginConfig = pluginConfig;
let route = getRoute(pluginConfig, file);

type ManifestRoute = Manifest["routes"][string];
Expand Down Expand Up @@ -1379,6 +1362,10 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
];
};

function isEqualJson(v1: unknown, v2: unknown) {
return JSON.stringify(v1) === JSON.stringify(v2);
}

function addRefreshWrapper(
pluginConfig: ResolvedRemixVitePluginConfig,
code: string,
Expand Down

0 comments on commit 3218847

Please sign in to comment.