Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unused Vite file system watcher #10510

Merged
merged 1 commit into from
Feb 28, 2025

Conversation

markdalgleish
Copy link
Member

@markdalgleish markdalgleish commented Feb 28, 2025

Fixes #10324.

This PR completely disables the vite-node file watcher because, even in dev, it's unused. The Vite dev server created for resolving routes.ts is only updated when the main dev server's file system watcher picks up a change. If the change is at all relevant to routes.ts, its module graph is completely invalidated. Relevant code:

viteDevServer.watcher.on("all", async (eventName, rawFilepath) => {
let { normalizePath } = getVite();
let filepath = normalizePath(rawFilepath);
let appFileAddedOrRemoved =
(eventName === "add" || eventName === "unlink") &&
filepath.startsWith(normalizePath(ctx.remixConfig.appDirectory));
invariant(viteConfig?.configFile);
let viteConfigChanged =
eventName === "change" &&
filepath === normalizePath(viteConfig.configFile);
let routeConfigChanged = Boolean(
routesViteNodeContext?.devServer?.moduleGraph.getModuleById(
filepath
)
);
if (routeConfigChanged || appFileAddedOrRemoved) {
routesViteNodeContext?.devServer?.moduleGraph.invalidateAll();
routesViteNodeContext?.runner?.moduleCache.clear();
}
if (
appFileAddedOrRemoved ||
viteConfigChanged ||
routeConfigChanged
) {
let lastRemixConfig = ctx.remixConfig;
await updateRemixPluginContext({ routeConfigChanged });
if (!isEqualJson(lastRemixConfig, ctx.remixConfig)) {
invalidateVirtualModules(viteDevServer);
}
}
});

We can be confident in this change because we have tests that check you can update a file in the routes.ts module graph and get a new route config during dev:

test("supports correcting an invalid route config module graph", async ({
page,
context,
browserName,
viteDev,
}) => {
let files: Files = async ({ port }) => ({
"vite.config.js": await viteConfig.basic({
routeConfig: true,
port,
}),
"app/routes.ts": js`
export { default } from "./actual-routes";
`,
"app/actual-routes.ts": js`
import { type RouteConfig, index } from "@remix-run/route-config";
export default [
index("test-route-1.tsx"),
] satisfies RouteConfig;
`,
"app/test-route-1.tsx": `
export default function TestRoute1() {
return <div data-test-route>Test route 1</div>
}
`,
"app/test-route-2.tsx": `
export default function TestRoute2() {
return <div data-test-route>Test route 2</div>
}
`,
});
let { cwd, port } = await viteDev(files, templateName);
await page.goto(`http://localhost:${port}/`, {
waitUntil: "networkidle",
});
await expect(page.locator("[data-test-route]")).toHaveText(
"Test route 1"
);
let edit = createEditor(cwd);
// Make config invalid
await edit("app/actual-routes.ts", (contents) => contents + "INVALID");
// Ensure dev server is still running with old config + HMR
await edit("app/test-route-1.tsx", (contents) =>
contents.replace("Test route 1", "Test route 1 updated")
);
await expect(page.locator("[data-test-route]")).toHaveText(
"Test route 1 updated"
);
// Fix config with new route
await edit("app/actual-routes.ts", (contents) =>
contents
.replace("INVALID", "")
.replace("test-route-1", "test-route-2")
);
await expect(async () => {
// Reload to pick up new route for current path
page = await reloadPage({ browserName, page, context });
await expect(page.locator("[data-test-route]")).toHaveText(
"Test route 2"
);
}).toPass();
});

Since the issue was caused by our use of vite.mergeConfig, I also simplified the code to avoid this API entirely.

Copy link

changeset-bot bot commented Feb 28, 2025

🦋 Changeset detected

Latest commit: e6fe273

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 19 packages
Name Type
@remix-run/dev Patch
@remix-run/fs-routes Patch
@remix-run/route-config Patch
@remix-run/routes-option-adapter Patch
create-remix Patch
remix Patch
@remix-run/architect Patch
@remix-run/cloudflare Patch
@remix-run/cloudflare-pages Patch
@remix-run/cloudflare-workers Patch
@remix-run/css-bundle Patch
@remix-run/deno Patch
@remix-run/eslint-config Patch
@remix-run/express Patch
@remix-run/node Patch
@remix-run/react Patch
@remix-run/serve Patch
@remix-run/server-runtime Patch
@remix-run/testing Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@markdalgleish markdalgleish merged commit 0b0dfaf into dev Feb 28, 2025
9 checks passed
@markdalgleish markdalgleish deleted the markdalgleish/remove-unused-file-watcher branch February 28, 2025 10:32
@github-actions github-actions bot added the awaiting release This issue has been fixed and will be released soon label Feb 28, 2025
Copy link
Contributor

🤖 Hello there,

We just published version 2.16.1-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

Copy link
Contributor

🤖 Hello there,

We just published version 2.16.1 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions github-actions bot removed the awaiting release This issue has been fixed and will be released soon label Mar 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant