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

Improve config change handling #12319

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from
5 changes: 5 additions & 0 deletions .changeset/stupid-kiwis-laugh.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@react-router/dev": patch
---

Update config when `react-router.config.ts` is created or deleted during development.
240 changes: 181 additions & 59 deletions packages/react-router-dev/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { execSync } from "node:child_process";
import PackageJson from "@npmcli/package-json";
import * as ViteNode from "../vite/vite-node";
import type * as Vite from "vite";
import path from "pathe";
import Path from "pathe";
import chokidar, {
type FSWatcher,
type EmitArgs as ChokidarEmitArgs,
Expand Down Expand Up @@ -419,14 +419,14 @@ async function resolveConfig({
);
}

let appDirectory = path.resolve(root, userAppDirectory || "app");
let buildDirectory = path.resolve(root, userBuildDirectory);
let appDirectory = Path.resolve(root, userAppDirectory || "app");
let buildDirectory = Path.resolve(root, userBuildDirectory);

let rootRouteFile = findEntry(appDirectory, "root");
if (!rootRouteFile) {
let rootRouteDisplayPath = path.relative(
let rootRouteDisplayPath = Path.relative(
root,
path.join(appDirectory, "root.tsx")
Path.join(appDirectory, "root.tsx")
);
return err(
`Could not find a root route module in the app directory as "${rootRouteDisplayPath}"`
Expand All @@ -441,17 +441,17 @@ async function resolveConfig({

try {
if (!routeConfigFile) {
let routeConfigDisplayPath = path.relative(
let routeConfigDisplayPath = Path.relative(
root,
path.join(appDirectory, "routes.ts")
Path.join(appDirectory, "routes.ts")
);
return err(`Route config file not found at "${routeConfigDisplayPath}".`);
}

setAppDirectory(appDirectory);
let routeConfigExport = (
await viteNodeContext.runner.executeFile(
path.join(appDirectory, routeConfigFile)
Path.join(appDirectory, routeConfigFile)
)
).default;
let routeConfig = await routeConfigExport;
Expand All @@ -476,7 +476,7 @@ async function resolveConfig({
"",
error.loc?.file && error.loc?.column && error.frame
? [
path.relative(appDirectory, error.loc.file) +
Path.relative(appDirectory, error.loc.file) +
":" +
error.loc.line +
":" +
Expand Down Expand Up @@ -526,7 +526,8 @@ type ChokidarEventName = ChokidarEmitArgs[0];

type ChangeHandler = (args: {
result: Result<ResolvedReactRouterConfig>;
configCodeUpdated: boolean;
configCodeChanged: boolean;
routeConfigCodeChanged: boolean;
configChanged: boolean;
routeConfigChanged: boolean;
path: string;
Expand All @@ -546,16 +547,27 @@ export async function createConfigLoader({
watch: boolean;
rootDirectory?: string;
}): Promise<ConfigLoader> {
root = root ?? process.env.REACT_ROUTER_ROOT ?? process.cwd();
root = Path.normalize(root ?? process.env.REACT_ROUTER_ROOT ?? process.cwd());

let vite = await import("vite");
let viteNodeContext = await ViteNode.createContext({
root,
mode: watch ? "development" : "production",
// Filter out any info level logs from vite-node
customLogger: vite.createLogger("warn", {
prefix: "[react-router]",
}),
});

let reactRouterConfigFile = findEntry(root, "react-router.config", {
absolute: true,
});
let reactRouterConfigFile: string | undefined;

let updateReactRouterConfigFile = () => {
reactRouterConfigFile = findEntry(root, "react-router.config", {
absolute: true,
});
};

updateReactRouterConfigFile();

let getConfig = () =>
resolveConfig({ root, viteNodeContext, reactRouterConfigFile });
Expand All @@ -568,9 +580,9 @@ export async function createConfigLoader({
throw new Error(initialConfigResult.error);
}

appDirectory = initialConfigResult.value.appDirectory;
appDirectory = Path.normalize(initialConfigResult.value.appDirectory);

let lastConfig = initialConfigResult.value;
let currentConfig = initialConfigResult.value;

let fsWatcher: FSWatcher | undefined;
let changeHandlers: ChangeHandler[] = [];
Expand All @@ -587,54 +599,108 @@ export async function createConfigLoader({
changeHandlers.push(handler);

if (!fsWatcher) {
fsWatcher = chokidar.watch(
[
...(reactRouterConfigFile ? [reactRouterConfigFile] : []),
appDirectory,
],
{ ignoreInitial: true }
);
fsWatcher = chokidar.watch([root, appDirectory], {
ignoreInitial: true,
ignored: (path) => {
let dirname = Path.dirname(path);

return (
!dirname.startsWith(appDirectory) &&
// Ensure we're only watching files outside of the app directory
// that are at the root level, not nested in subdirectories
path !== root && // Watch the root directory itself
dirname !== root // Watch files at the root level
Comment on lines +608 to +612
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, we could watch the root for only changes to <root>/react-router.config.ts (and its module graph) and the corresponding app directory, rather than coarse-grained allowlist for any file in the root. But that's not a blocker.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I think this would be relatively easy to do with Vite Node's .devserver.watcher.on

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we've since disabled the watcher for the Vite Node context. But my point still stands: its better to watch for changes in the app directory and the React Router config directly, rather than watching the root and ignoring paths.

We'd need to take care to add any files that are part of the module graph for the React Router config to the watched files and then clean them up as needed. So there is a bit of complexity to get this working.

);
},
});

fsWatcher.on("all", async (...args: ChokidarEmitArgs) => {
let [event, rawFilepath] = args;
let filepath = path.normalize(rawFilepath);
let filepath = Path.normalize(rawFilepath);

let fileAddedOrRemoved = event === "add" || event === "unlink";

let appFileAddedOrRemoved =
appDirectory &&
(event === "add" || event === "unlink") &&
filepath.startsWith(path.normalize(appDirectory));
fileAddedOrRemoved &&
filepath.startsWith(Path.normalize(appDirectory));

let configCodeUpdated = Boolean(
viteNodeContext.devServer?.moduleGraph.getModuleById(filepath)
);
let rootRelativeFilepath = Path.relative(root, filepath);

let configFileAddedOrRemoved =
fileAddedOrRemoved &&
isEntryFile("react-router.config", rootRelativeFilepath);

if (configCodeUpdated || appFileAddedOrRemoved) {
viteNodeContext.devServer?.moduleGraph.invalidateAll();
viteNodeContext.runner?.moduleCache.clear();
if (configFileAddedOrRemoved) {
updateReactRouterConfigFile();
}

if (appFileAddedOrRemoved || configCodeUpdated) {
let result = await getConfig();
let moduleGraphChanged =
configFileAddedOrRemoved ||
Boolean(
viteNodeContext.devServer?.moduleGraph.getModuleById(filepath)
);

let configChanged = result.ok && !isEqual(lastConfig, result.value);
// Bail out if no relevant changes detected
if (!moduleGraphChanged && !appFileAddedOrRemoved) {
return;
}

viteNodeContext.devServer?.moduleGraph.invalidateAll();
viteNodeContext.runner?.moduleCache.clear();

let result = await getConfig();

let prevAppDirectory = appDirectory;
appDirectory = Path.normalize(
(result.value ?? currentConfig).appDirectory
);

let routeConfigChanged =
result.ok && !isEqual(lastConfig?.routes, result.value.routes);
if (appDirectory !== prevAppDirectory) {
fsWatcher!.unwatch(prevAppDirectory);
fsWatcher!.add(appDirectory);
}

for (let handler of changeHandlers) {
handler({
result,
configCodeUpdated,
configChanged,
routeConfigChanged,
path: filepath,
event,
});
}
let configCodeChanged =
configFileAddedOrRemoved ||
(reactRouterConfigFile !== undefined &&
isEntryFileDependency(
viteNodeContext.devServer.moduleGraph,
reactRouterConfigFile,
filepath
));

let routeConfigFile = findEntry(appDirectory, "routes", {
absolute: true,
});
let routeConfigCodeChanged =
routeConfigFile !== undefined &&
isEntryFileDependency(
viteNodeContext.devServer.moduleGraph,
routeConfigFile,
filepath
);

let configChanged =
result.ok &&
!isEqual(omitRoutes(currentConfig), omitRoutes(result.value));

let routeConfigChanged =
result.ok && !isEqual(currentConfig?.routes, result.value.routes);

for (let handler of changeHandlers) {
handler({
result,
configCodeChanged,
routeConfigCodeChanged,
configChanged,
routeConfigChanged,
path: filepath,
event,
});
}

if (result.ok) {
lastConfig = result.value;
}
if (result.ok) {
currentConfig = result.value;
}
});
}
Expand Down Expand Up @@ -672,8 +738,8 @@ export async function resolveEntryFiles({
}) {
let { appDirectory } = reactRouterConfig;

let defaultsDirectory = path.resolve(
path.dirname(require.resolve("@react-router/dev/package.json")),
let defaultsDirectory = Path.resolve(
Path.dirname(require.resolve("@react-router/dev/package.json")),
"dist",
"config",
"defaults"
Expand Down Expand Up @@ -723,29 +789,85 @@ export async function resolveEntryFiles({
}

let entryClientFilePath = userEntryClientFile
? path.resolve(reactRouterConfig.appDirectory, userEntryClientFile)
: path.resolve(defaultsDirectory, entryClientFile);
? Path.resolve(reactRouterConfig.appDirectory, userEntryClientFile)
: Path.resolve(defaultsDirectory, entryClientFile);

let entryServerFilePath = userEntryServerFile
? path.resolve(reactRouterConfig.appDirectory, userEntryServerFile)
: path.resolve(defaultsDirectory, entryServerFile);
? Path.resolve(reactRouterConfig.appDirectory, userEntryServerFile)
: Path.resolve(defaultsDirectory, entryServerFile);

return { entryClientFilePath, entryServerFilePath };
}

function omitRoutes(
config: ResolvedReactRouterConfig
): ResolvedReactRouterConfig {
return {
...config,
routes: {},
};
}

const entryExts = [".js", ".jsx", ".ts", ".tsx"];

function isEntryFile(entryBasename: string, filename: string) {
return entryExts.some((ext) => filename === `${entryBasename}${ext}`);
}

function findEntry(
dir: string,
basename: string,
options?: { absolute?: boolean }
): string | undefined {
for (let ext of entryExts) {
let file = path.resolve(dir, basename + ext);
let file = Path.resolve(dir, basename + ext);
if (fs.existsSync(file)) {
return options?.absolute ?? false ? file : path.relative(dir, file);
return options?.absolute ?? false ? file : Path.relative(dir, file);
}
}

return undefined;
}

function isEntryFileDependency(
moduleGraph: Vite.ModuleGraph,
entryFilepath: string,
filepath: string,
visited = new Set<string>()
): boolean {
// Ensure normalized paths
entryFilepath = Path.normalize(entryFilepath);
filepath = Path.normalize(filepath);

if (visited.has(filepath)) {
return false;
}

visited.add(filepath);

if (filepath === entryFilepath) {
return true;
}

let mod = moduleGraph.getModuleById(filepath);

if (!mod) {
return false;
}

// Recursively check all importers to see if any of them are the entry file
for (let importer of mod.importers) {
if (!importer.id) {
continue;
}

if (
importer.id === entryFilepath ||
isEntryFileDependency(moduleGraph, entryFilepath, importer.id, visited)
) {
return true;
}
}

return false;
}
Loading
Loading