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

[Bug]: react-router-serve requires cross-env NODE_ENV=production #12078

Open
jrestall opened this issue Oct 6, 2024 · 5 comments
Open

[Bug]: react-router-serve requires cross-env NODE_ENV=production #12078

jrestall opened this issue Oct 6, 2024 · 5 comments
Labels

Comments

@jrestall
Copy link

jrestall commented Oct 6, 2024

What version of React Router are you using?

7.0.0-pre.0

Steps to Reproduce

git clone https://github.com/jacobparis/underkill-stack.git
pnpm install
pnpm build
edit package.json to remove NODE_ENV=production from the "start" script.
pnpm start

HTTP 500: Internal Server Error -> "Unexpected Server Error"

The userland fix for the error is to always specify NODE_ENV=production in the package.json start script.

- "start": "react-router-serve ./build/server/index.js"
+ "start": "cross-env NODE_ENV=production react-router-serve ./build/server/index.js"

This could be because whilst react-router-serve tries to default to NODE_ENV=production, react is loaded earlier than when that code runs and loads the development version of react in the absence of a NODE_ENV env variable. Then later the app server build is dynamically imported and subsequently imports react-dom, but now NODE_ENV has been set to production by react-router-serve, so there is a mismatch in the loaded react libraries with a mix of development and production.

process.env.NODE_ENV = process.env.NODE_ENV ?? "production";

The import chain that results in the react development version being loaded is:

node_modules/@react-router/serve/dist/cli.js
-> @react-router/node
-> node_modules/@react-router/node/dist/index.js
-> node_modules/@react-router/node/dist/sessions/fileStorage.js
-> node_modules/react-router/dist/main.js
-> node_modules/react/index.js
// node_modules/react/index.js
console.log(process.env.NODE_ENV); // -> undefined
if (process.env.NODE_ENV === 'production') {
  module.exports = require('./cjs/react.production.js');
} else {
  module.exports = require('./cjs/react.development.js');
}
// node_modules/react-dom/index.js
console.log(process.env.NODE_ENV); // -> production
if (process.env.NODE_ENV === 'production') {
  module.exports = require('./cjs/react.production.js');
} else {
  module.exports = require('./cjs/react.development.js');
}

So should we always pass a NODE_ENV in the start script or is this an issue?

System Info

Node: v21.1.0
OS: Mac

Expected Behavior

Site returns 200 response.

Actual Behavior

[react-router-serve] http://localhost:3000 (http://192.168.20.16:3000)
TypeError: dispatcher.getOwner is not a function
    at getOwner (/Users/jrestall/Dev/underkill-stack/node_modules/.pnpm/[email protected]/node_modules/react/cjs/react.development.js:412:54)
    at Module.createElement (/Users/jrestall/Dev/underkill-stack/node_modules/.pnpm/[email protected]/node_modules/react/cjs/react.development.js:1331:9)
    at mapRouteProperties (file:///Users/jrestall/Dev/underkill-stack/node_modules/.pnpm/[email protected][email protected][email protected]_bnl2udvc6mytkjbq3r7oxas3x4/node_modules/lib/components.tsx:93:22)
    at map (file:///Users/jrestall/Dev/underkill-stack/node_modules/.pnpm/[email protected][email protected][email protected]_bnl2udvc6mytkjbq3r7oxas3x4/node_modules/lib/router/utils.ts:454:12)
    at Array.map (<anonymous>)
    at convertRoutesToDataRoutes (file:///Users/jrestall/Dev/underkill-stack/node_modules/.pnpm/[email protected][email protected][email protected]_bnl2udvc6mytkjbq3r7oxas3x4/node_modules/lib/router/utils.ts:430:17)
    at createStaticRouter (file:///Users/jrestall/Dev/underkill-stack/node_modules/.pnpm/[email protected][email protected][email protected]_bnl2udvc6mytkjbq3r7oxas3x4/node_modules/lib/dom/server.tsx:281:20)
    at ServerRouter (file:///Users/jrestall/Dev/underkill-stack/node_modules/.pnpm/[email protected][email protected][email protected]_bnl2udvc6mytkjbq3r7oxas3x4/node_modules/lib/dom/ssr/server.tsx:68:16)
    at renderWithHooks (/Users/jrestall/Dev/underkill-stack/node_modules/.pnpm/[email protected][email protected]/node_modules/react-dom/cjs/react-dom-server.node.production.js:3769:18)
    at renderElement (/Users/jrestall/Dev/underkill-stack/node_modules/.pnpm/[email protected][email protected]/node_modules/react-dom/cjs/react-dom-server.node.production.js:3907:14)
@jrestall jrestall added the bug label Oct 6, 2024
@jacobparis
Copy link

I think I agree that react-router should match react's behaviour in cases where NODE_ENV is not set

though in a vacuum I do think loading the production build would have been a better default for both

@brophdawg11
Copy link
Contributor

What is the ask here? To add that to the template? Or to Jacob's underkill stack?

If the former, would you care to submit a PR?

@jacobparis
Copy link

I think the ask is for react-router-serve to not set NODE_ENV to production, because react router cannot tell react to run the production build and therefore will be out of sync with it

@jrestall
Copy link
Author

What is the ask here? To add that to the template? Or to Jacob's underkill stack?

If the former, would you care to submit a PR?

Neither, I'd prefer devs don't need to even think about it and we handle setting node_env automatically in packages/react-router-serve/bin.js to ensure it is consistent and set before any react module is loaded by the react-router-serve cli code.

Here's my proposed PR fix for the bin.js change #12193.

I also found this error is only reproducible with React v19 but even with React v18 the dev react and prod react-dom modules are loaded, it just doesn't hard error. So I think still a good idea to fix.

@brophdawg11
Copy link
Contributor

I'd prefer aligning our default with Reacts - but will run this by the team. The side-effect nature of the linked PR could make it very tricky to diagnose why React was loading the production build if you were never setting NODE_ENV in your environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants