Fix react-compiler entrypoint for react-server #29057
Conversation
Adds a `react-compiler-runtime.react-server` entrypoint. We need a separate "react-server" build, because otherwise the compiler runtime attempts to use `__CLIENT_INTERNALS` even under the "react-server" condition. AFAICT this is because in a server environment, we need `shared/ReactSharedInternals` to be rewritten, but unless a "react-server" condition is passed to the build, it'll default to `ReactSharedInternalsClient`.
There was a problem hiding this comment.
Alternatively, this could be a stub file like
throw new Error("The React compiler does not support Server Components.");because AFAIK RSC's are not currently intended to be processed by the compiler at all
But also, the Flight dispatcher does have a stub useMemoCache implementation, so i'm not sure what's the preferred behavior here
There was a problem hiding this comment.
But also, the Flight dispatcher does have a stub useMemoCache implementation
We should remove it.
There was a problem hiding this comment.
A throwing stub is the way.
https://github.com/facebook/react/blob/main/packages/react-dom/npm/server.react-server.js#L3-L5
This export was previously on the main react runtime and that was the reason we couldn't do that. However, if the React Compiler starts supporting some modes in the RSC mode such as optimizing RSC rendering this wouldn't work again.
We have as a principle that we should follow ESM so that there can be an early compiler error if an ESM export is missing. This can catch errors earlier without having to run some code. Erroring early is more important than error with a nice message. It also provides the option to conditionally check for the existence of a method if you import it in the other way that it doesn't early error. E.g. this is why "react" in an RSC layer doesn't export a dummy useState function.
In this case it doesn't harm adding the throw in the module initializer but we might have to drop that once we start having react-compiler support for RSC things. At the end of the day, whoever is configuring an advanced set up like a compiler isn't our main target for nice error messages so it's not the end of the world.
|
@sebmarkbage Happy to change it, but i see one argument for making this a noop instead of an error-stub that I think we should consider first If an NPM package exports a universal (server + client) component, and starts compiling its code with the compiler (IIRC shipping compiled code is the recommendation?), then the error-stub would suddenly make importing that component in a server context impossible, because it'd instantly throw. Fixing the error would then require one of these:
So if people aren't careful, using the compiler in a package can become an accidental breaking change, causing general frustration. I definitely understand the desire to be strict here, but I feel like this could end up causing unnecessary ecosystem churn. So maybe it's worth being more lenient and no-oping instead, if we can? Or would you consider that papering over a potential bug, and thus undesireable? |
|
That's expected though because compiled code might not work with RSC. There's no guarantee that the compiled output would run with any kind of noop memoization and it's not prepared for what we have planned for the JSX runtime inside the RSC layer. The "RSC Compiler" would not work for client code. So by breaking it now we're ensuring that it keeps working in the contexts that it used to work in the future and doesn't just break in a minor. The solution for a library that wants to use everything is a more complex build set up but that's the case regardless. |
|
More than that, I don't think the compiler story for SSR is fully figured out. E.g. we should really have an SSR compiler that can produce more "string concat" style optimizations and we plan on it excluding things like useEffect and inlining useState. The React Compiler as is now really shouldn't be applied to the SSR layer so if published to npm with the compiled output it really shouldn't be used with SSR. That story doesn't quite make sense yet. |
|
EDIT: disregard this. i thought i found a thing, but i think i misunderstood the compiler code. This doesn't actually do what that comment says, because
|
|
Thanks for posting this. I think we need to discuss the path forward for React Compiler and SSR/RSC a bit more before proceeding. |
Fixes a stale comment since this caused confusion in #29057 (comment) [ghstack-poisoned]
Summary
Currently, if
react/compiler-runtimeis imported under "react-server" andc()(a.k.auseMemoCache) is called, it'll crash with"Cannot read properties of undefined (reading 'H')".Instead, we'd likely want it to 1. work correctly or 2. print a sensible error message.
This happens because the current
react-compiler-runtime.{development,production}.jsends up trying to read__CLIENT_INTERNALS, which obviously doesn't exist inreact.react-server.The fix is to add a new
react-compiler-runtime.react-serverbundle, built with the "react-server" condition, which will make imports ofshared/ReactSharedInternalsresolve to the server internals instead.How did you test this change?
Not sure what the best way to test this is, but the following command previously crashed with
"Cannot read properties of undefined (reading 'H')". after the change it properly warns about a missing hook call (because i'm calling it outside of render)