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

Decouple CLI arg parsing/execution for NODE_ENV initialization #12505

Closed
wants to merge 11 commits into from
6 changes: 6 additions & 0 deletions .changeset/default-node-env.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@react-router/serve": patch
"@react-router/dev": patch
---

Properly initialize NODE_ENV if not set for compatibility with React 19
brophdawg11 marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions contributors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@
- johnpangalos
- jonkoops
- jrakotoharisoa
- jrestall
- juanpprieto
- jungwoo3490
- kachun333
Expand Down
33 changes: 23 additions & 10 deletions packages/react-router-dev/cli/index.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,25 @@
#!/usr/bin/env node
import { run } from "./run";
import { parseArgs } from "./parse";

run().then(
() => {
process.exit(0);
},
(error: unknown) => {
if (error) console.error(error);
process.exit(1);
}
);
let { input, flags, command } = parseArgs();

// If not already set, default `NODE_ENV` so React loads the proper
// version in it's CJS entry script. We have to do this before importing `run.ts`
// since that is what imports `react` (indirectly via `react-router`)
if (command === "dev") {
process.env.NODE_ENV = process.env.NODE_ENV ?? "development";
} else {
process.env.NODE_ENV = process.env.NODE_ENV ?? "production";
}
Comment on lines +5 to +14
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Parse args and set NODE_ENV first


import("./run").then(({ run }) => {
run(input, flags, command).then(
() => {
process.exit(0);
},
(error: unknown) => {
if (error) console.error(error);
process.exit(1);
}
);
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then run the CLI

85 changes: 85 additions & 0 deletions packages/react-router-dev/cli/parse.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import arg from "arg";
import semver from "semver";

/**
* Parse command line arguments for `react-router` dev CLI
*/
export function parseArgs(argv: string[] = process.argv.slice(2)) {
// Check the node version
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No changes to this logic, just extracted out from run() below

let versions = process.versions;
let MINIMUM_NODE_VERSION = 20;
if (
versions &&
versions.node &&
semver.major(versions.node) < MINIMUM_NODE_VERSION
) {
console.warn(
`️⚠️ Oops, Node v${versions.node} detected. react-router requires ` +
`a Node version greater than ${MINIMUM_NODE_VERSION}.`
);
}

let isBooleanFlag = (arg: string) => {
let index = argv.indexOf(arg);
let nextArg = argv[index + 1];
return !nextArg || nextArg.startsWith("-");
};

let args = arg(
{
"--force": Boolean,
"--help": Boolean,
"-h": "--help",
"--json": Boolean,
"--token": String,
"--typescript": Boolean,
"--no-typescript": Boolean,
"--version": Boolean,
"-v": "--version",
"--port": Number,
"-p": "--port",
"--config": String,
"-c": "--config",
"--assetsInlineLimit": Number,
"--clearScreen": Boolean,
"--cors": Boolean,
"--emptyOutDir": Boolean,
"--host": isBooleanFlag("--host") ? Boolean : String,
"--logLevel": String,
"-l": "--logLevel",
"--minify": String,
"--mode": String,
"-m": "--mode",
"--open": isBooleanFlag("--open") ? Boolean : String,
"--strictPort": Boolean,
"--profile": Boolean,
"--sourcemapClient": isBooleanFlag("--sourcemapClient")
? Boolean
: String,
"--sourcemapServer": isBooleanFlag("--sourcemapServer")
? Boolean
: String,
"--watch": Boolean,
},
{
argv,
}
);

let flags: any = Object.entries(args).reduce((acc, [key, value]) => {
key = key.replace(/^--/, "");
acc[key] = value;
return acc;
}, {} as any);

flags.interactive = flags.interactive ?? require.main === module;
if (args["--no-typescript"]) {
flags.typescript = false;
}

return {
input: args._,
command: args._[0],
flags,
};
}
80 changes: 1 addition & 79 deletions packages/react-router-dev/cli/run.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import arg from "arg";
import semver from "semver";
import colors from "picocolors";

import * as commands from "./commands";
Expand Down Expand Up @@ -81,76 +79,7 @@ ${colors.blueBright("react-router")}
* Programmatic interface for running the react-router CLI with the given command line
* arguments.
*/
export async function run(argv: string[] = process.argv.slice(2)) {
// Check the node version
let versions = process.versions;
let MINIMUM_NODE_VERSION = 20;
if (
versions &&
versions.node &&
semver.major(versions.node) < MINIMUM_NODE_VERSION
) {
console.warn(
`️⚠️ Oops, Node v${versions.node} detected. react-router requires ` +
`a Node version greater than ${MINIMUM_NODE_VERSION}.`
);
}

let isBooleanFlag = (arg: string) => {
let index = argv.indexOf(arg);
let nextArg = argv[index + 1];
return !nextArg || nextArg.startsWith("-");
};

let args = arg(
{
"--force": Boolean,
"--help": Boolean,
"-h": "--help",
"--json": Boolean,
"--token": String,
"--typescript": Boolean,
"--no-typescript": Boolean,
"--version": Boolean,
"-v": "--version",
"--port": Number,
"-p": "--port",
"--config": String,
"-c": "--config",
"--assetsInlineLimit": Number,
"--clearScreen": Boolean,
"--cors": Boolean,
"--emptyOutDir": Boolean,
"--host": isBooleanFlag("--host") ? Boolean : String,
"--logLevel": String,
"-l": "--logLevel",
"--minify": String,
"--mode": String,
"-m": "--mode",
"--open": isBooleanFlag("--open") ? Boolean : String,
"--strictPort": Boolean,
"--profile": Boolean,
"--sourcemapClient": isBooleanFlag("--sourcemapClient")
? Boolean
: String,
"--sourcemapServer": isBooleanFlag("--sourcemapServer")
? Boolean
: String,
"--watch": Boolean,
},
{
argv,
}
);

let input = args._;

let flags: any = Object.entries(args).reduce((acc, [key, value]) => {
key = key.replace(/^--/, "");
acc[key] = value;
return acc;
}, {} as any);

export async function run(input: string[], flags: any, command: string) {
if (flags.help) {
console.log(helpText);
return;
Expand All @@ -161,13 +90,6 @@ export async function run(argv: string[] = process.argv.slice(2)) {
return;
}

flags.interactive = flags.interactive ?? require.main === module;
if (args["--no-typescript"]) {
flags.typescript = false;
}

let command = input[0];

// Note: Keep each case in this switch statement small.
switch (command) {
case "routes":
Expand Down
1 change: 1 addition & 0 deletions packages/react-router-dev/tsup.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import pkg from "./package.json";

const entry = [
"cli/index.ts",
"cli/run.ts",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needed to make run it's own entry point so it can be lazily imported as runtime

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hm, well that didn't fix CI so may need to dig into this part a bit deeper

"config.ts",
"routes.ts",
"vite.ts",
Expand Down
5 changes: 5 additions & 0 deletions packages/react-router-serve/bin.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,7 @@
#!/usr/bin/env node

// If not already set, default `NODE_ENV=production` so React loads the proper
// version in it's CJS entry script
process.env.NODE_ENV = process.env.NODE_ENV ?? "production";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Set this correctly before calling the CLI so the proper version of React gets loaded (#12078)


require("./dist/cli");
Loading