remix-run / react-router

Declarative routing for React
https://reactrouter.com
MIT License
52.9k stars 10.24k forks source link

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

Open jrestall opened 22 hours ago

jrestall commented 22 hours ago

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
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. https://github.com/remix-run/react-router/blob/bc1c1c8ef87b7bf97f1f168604b89a5da61939da/packages/react-router-serve/cli.ts#L15

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/react@19.0.0-rc-1460d67c-20241003/node_modules/react/cjs/react.development.js:412:54)
    at Module.createElement (/Users/jrestall/Dev/underkill-stack/node_modules/.pnpm/react@19.0.0-rc-1460d67c-20241003/node_modules/react/cjs/react.development.js:1331:9)
    at mapRouteProperties (file:///Users/jrestall/Dev/underkill-stack/node_modules/.pnpm/react-router@7.0.0-pre.0_react-dom@19.0.0-rc-1460d67c-20241003_react@19.0.0-rc-1460d67c-20241_bnl2udvc6mytkjbq3r7oxas3x4/node_modules/lib/components.tsx:93:22)
    at map (file:///Users/jrestall/Dev/underkill-stack/node_modules/.pnpm/react-router@7.0.0-pre.0_react-dom@19.0.0-rc-1460d67c-20241003_react@19.0.0-rc-1460d67c-20241_bnl2udvc6mytkjbq3r7oxas3x4/node_modules/lib/router/utils.ts:454:12)
    at Array.map (<anonymous>)
    at convertRoutesToDataRoutes (file:///Users/jrestall/Dev/underkill-stack/node_modules/.pnpm/react-router@7.0.0-pre.0_react-dom@19.0.0-rc-1460d67c-20241003_react@19.0.0-rc-1460d67c-20241_bnl2udvc6mytkjbq3r7oxas3x4/node_modules/lib/router/utils.ts:430:17)
    at createStaticRouter (file:///Users/jrestall/Dev/underkill-stack/node_modules/.pnpm/react-router@7.0.0-pre.0_react-dom@19.0.0-rc-1460d67c-20241003_react@19.0.0-rc-1460d67c-20241_bnl2udvc6mytkjbq3r7oxas3x4/node_modules/lib/dom/server.tsx:281:20)
    at ServerRouter (file:///Users/jrestall/Dev/underkill-stack/node_modules/.pnpm/react-router@7.0.0-pre.0_react-dom@19.0.0-rc-1460d67c-20241003_react@19.0.0-rc-1460d67c-20241_bnl2udvc6mytkjbq3r7oxas3x4/node_modules/lib/dom/ssr/server.tsx:68:16)
    at renderWithHooks (/Users/jrestall/Dev/underkill-stack/node_modules/.pnpm/react-dom@19.0.0-rc-1460d67c-20241003_react@19.0.0-rc-1460d67c-20241003/node_modules/react-dom/cjs/react-dom-server.node.production.js:3769:18)
    at renderElement (/Users/jrestall/Dev/underkill-stack/node_modules/.pnpm/react-dom@19.0.0-rc-1460d67c-20241003_react@19.0.0-rc-1460d67c-20241003/node_modules/react-dom/cjs/react-dom-server.node.production.js:3907:14)
jacobparis commented 20 hours ago

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