Skip to content

Fix root directory resolution logic #13472

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

Merged
merged 1 commit into from
Apr 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/bright-experts-grab.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@react-router/dev": patch
---

Support project root directories without a `package.json` if it exists in a parent directory
5 changes: 5 additions & 0 deletions .changeset/cold-seals-count.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@react-router/dev": patch
---

When providing a custom Vite config path via the CLI `--config`/`-c` flag, default the project root directory to the directory containing the Vite config when not explicitly provided
5 changes: 5 additions & 0 deletions .changeset/forty-ants-teach.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@react-router/dev": patch
---

Ensure consistent project root directory resolution logic in CLI commands
41 changes: 29 additions & 12 deletions packages/react-router-dev/cli/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ import * as Typegen from "../typegen";
import { preloadVite, getVite } from "../vite/vite";

export async function routes(
reactRouterRoot?: string,
rootDirectory?: string,
flags: {
config?: string;
json?: boolean;
} = {}
): Promise<void> {
let rootDirectory = reactRouterRoot ?? process.cwd();
rootDirectory = resolveRootDirectory(rootDirectory, flags);
let configResult = await loadConfig({ rootDirectory });

if (!configResult.ok) {
Expand All @@ -39,9 +39,7 @@ export async function build(
root?: string,
options: ViteBuildOptions = {}
): Promise<void> {
if (!root) {
root = process.env.REACT_ROUTER_ROOT || process.cwd();
}
root = resolveRootDirectory(root, options);

let { build } = await import("../vite/build");
if (options.profile) {
Expand All @@ -54,12 +52,14 @@ export async function build(
}
}

export async function dev(root: string, options: ViteDevOptions = {}) {
export async function dev(root?: string, options: ViteDevOptions = {}) {
let { dev } = await import("../vite/dev");
if (options.profile) {
await profiler.start();
}
exitHook(() => profiler.stop(console.info));

root = resolveRootDirectory(root, options);
await dev(root, options);

// keep `react-router dev` alive by waiting indefinitely
Expand All @@ -77,20 +77,20 @@ let conjunctionListFormat = new Intl.ListFormat("en", {

export async function generateEntry(
entry?: string,
reactRouterRoot?: string,
rootDirectory?: string,
flags: {
typescript?: boolean;
config?: string;
} = {}
) {
// if no entry passed, attempt to create both
if (!entry) {
await generateEntry("entry.client", reactRouterRoot, flags);
await generateEntry("entry.server", reactRouterRoot, flags);
await generateEntry("entry.client", rootDirectory, flags);
await generateEntry("entry.server", rootDirectory, flags);
return;
}

let rootDirectory = reactRouterRoot ?? process.cwd();
rootDirectory = resolveRootDirectory(rootDirectory, flags);
let configResult = await loadConfig({ rootDirectory });

if (!configResult.ok) {
Expand Down Expand Up @@ -162,6 +162,17 @@ export async function generateEntry(
);
}

function resolveRootDirectory(root?: string, flags?: { config?: string }) {
if (root) {
return path.resolve(root);
Comment on lines +166 to +167
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that if a root directory was explicitly provided (e.g. react-router build my/app/dir) this will still take precedence. We only fall back to inferring the root when this isn't provided, so anyone relying on this should maintain existing behaviour.

}

return (
process.env.REACT_ROUTER_ROOT ||
(flags?.config ? path.dirname(path.resolve(flags.config)) : process.cwd())
);
}

async function checkForEntry(
rootDirectory: string,
appDirectory: string,
Expand Down Expand Up @@ -198,8 +209,14 @@ async function createClientEntry(
return contents;
}

export async function typegen(root: string, flags: { watch: boolean }) {
root ??= process.cwd();
export async function typegen(
root: string,
flags: {
watch?: boolean;
config?: string;
Copy link
Member Author

@markdalgleish markdalgleish Apr 28, 2025

Choose a reason for hiding this comment

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

@pcattori Just calling out that since the --config flag impacts the project root directory (and hence where routes live etc.), I think it makes sense to support this in all CLI commands for consistency, even those that don't use the Vite config.

Copy link
Member Author

@markdalgleish markdalgleish Apr 28, 2025

Choose a reason for hiding this comment

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

Side note, but I think this raises something I might have missed when we introduced react-router.config.ts, which is that the --config flag probably should have been updated to point to our config, and any custom Vite config path should have been configured there, or via a separate --vite-config flag.

This is part of my rationale for this change. If our --config flag pointed to react-router.config.ts, I would want to support it here too.

}
) {
root = resolveRootDirectory(root, flags);

if (flags.watch) {
await preloadVite();
Expand Down
51 changes: 42 additions & 9 deletions packages/react-router-dev/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,20 @@ export async function resolveEntryFiles({
let entryServerFile: string;
let entryClientFile = userEntryClientFile || "entry.client.tsx";

let pkgJson = await PackageJson.load(rootDirectory);
let packageJsonPath = findEntry(rootDirectory, "package", {
extensions: [".json"],
absolute: true,
walkParents: true,
});

if (!packageJsonPath) {
throw new Error(
`Could not find package.json in ${rootDirectory} or any of its parent directories`
);
}

let packageJsonDirectory = path.dirname(packageJsonPath);
let pkgJson = await PackageJson.load(packageJsonDirectory);
let deps = pkgJson.content.dependencies ?? {};

if (userEntryServerFile) {
Expand Down Expand Up @@ -717,7 +730,7 @@ export async function resolveEntryFiles({
let packageManager = detectPackageManager() ?? "npm";

execSync(`${packageManager} install`, {
cwd: rootDirectory,
cwd: packageJsonDirectory,
stdio: "inherit",
});
}
Expand All @@ -741,14 +754,34 @@ const entryExts = [".js", ".jsx", ".ts", ".tsx"];
function findEntry(
dir: string,
basename: string,
options?: { absolute?: boolean }
options?: {
absolute?: boolean;
extensions?: string[];
walkParents?: boolean;
}
): string | undefined {
for (let ext of entryExts) {
let file = path.resolve(dir, basename + ext);
if (fs.existsSync(file)) {
return options?.absolute ?? false ? file : path.relative(dir, file);
let currentDir = path.resolve(dir);
let { root } = path.parse(currentDir);

while (true) {
for (let ext of options?.extensions ?? entryExts) {
let file = path.resolve(currentDir, basename + ext);
if (fs.existsSync(file)) {
return options?.absolute ?? false ? file : path.relative(dir, file);
}
}

if (!options?.walkParents) {
return undefined;
}
}

return undefined;
let parentDir = path.dirname(currentDir);
// Break out when we've reached the root directory or we're about to get
// stuck in a loop where `path.dirname` keeps returning "/"
if (currentDir === root || parentDir === currentDir) {
return undefined;
}

currentDir = parentDir;
}
}