Skip to content

Commit

Permalink
[Flight] Encode React Elements in Replies as Temporary References (#2…
Browse files Browse the repository at this point in the history
…8564)

Currently you can accidentally pass React Element to a Server Action. It
warns but in prod it actually works because we can encode the symbol and
otherwise it's mostly a plain object. It only works if you only pass
host components and no function props etc. which makes it potentially
error later. The first thing this does it just early hard error for
elements.

I made Lazy work by unwrapping though since that will be replaced by
Promises later which works.

Our protocol is not fully symmetric in that elements flow from Server ->
Client. Only the Server can resolve Components and only the client
should really be able to receive host components. It's not intended that
a Server can actually do something with them other than passing them to
the client.

In the case of a Reply, we expect the client to be stateful. It's
waiting for a response. So anything we can't serialize we can still pass
by reference to an in memory object. So I introduce the concept of a
TemporaryReferenceSet which is an opaque object that you create before
encoding the reply. This then stashes any unserializable values in this
set and encode the slot by id. When a new response from the Action then
returns we pass the same temporary set into the parser which can then
restore the objects. This lets you pass a value by reference to the
server and back into another slot.

For example it can be used to render children inside a parent tree from
a server action:

```
export async function Component({ children }) {
  "use server";
  return <div>{children}</div>;
}
```

(You wouldn't normally do this due to the waterfalls but for advanced
cases.)

A common scenario where this comes up accidentally today is in
`useActionState`.

```
export function action(state, formData) {
  "use server";
   if (errored) {
     return <div>This action <strong>errored</strong></div>;
   }
   return null;
}
```

```
const [errors, formAction] = useActionState(action);
return <div>{errors}<div>;
```

It feels like I'm just passing the JSX from server to client. However,
because `useActionState` also sends the previous state *back* to the
server this should not actually be valid. Before this PR this actually
worked accidentally. You get a DEV warning but it used to work in prod.
Once you do something like pass a client reference it won't work tho. We
could perhaps make client references work by stashing where we got them
from but it wouldn't work with all possible JSX.

By adding temporary references to the action implementation this will
work again - on the client. It'll also be more efficient since we don't
send back the JSX content that you shouldn't introspect on the server
anyway.

However, a flaw here is that the progressive enhancement of this case
won't work because we can't use temporary references for progressive
enhancement since there's no in memory stash. What is worse is that it
won't error if you hydrate. ~It also will error late in the example
above because the first state is "undefined" so invoking the form once
works - it errors on the second attempt when it tries to send the error
state back again.~ It actually errors on the first invocation because we
need to eagerly serialize "previous state" into the form. So at least
that's better.

I think maybe the solution to this particular pattern would be to allow
JSX to serialize if you have no temporary reference set, and remember
client references so that client references can be returned back to the
server as client references. That way anything you could send from the
server could also be returned to the server. But it would only deopt to
serializing it for progressive enhancement. The consequence of that
would be that there's a lot of JSX that might accidentally seem like it
should work but it's only if you've gotten it from the server before
that it works. This would have to have pair them somehow though since
you can't take a client reference from one implementation of Flight and
use it with another.

DiffTrain build for commit 83409a1.
  • Loading branch information
sebmarkbage committed Mar 19, 2024
1 parent ebb02e9 commit 8810d83
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9175,7 +9175,7 @@ var devToolsConfig$jscomp$inline_1016 = {
throw Error("TestRenderer does not support findFiberByHostInstance()");
},
bundleType: 0,
version: "18.3.0-canary-cb076b593-20240319",
version: "18.3.0-canary-83409a1fd-20240319",
rendererPackageName: "react-test-renderer"
};
var internals$jscomp$inline_1194 = {
Expand Down Expand Up @@ -9206,7 +9206,7 @@ var internals$jscomp$inline_1194 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-canary-cb076b593-20240319"
reconcilerVersion: "18.3.0-canary-83409a1fd-20240319"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_1195 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9603,7 +9603,7 @@ var devToolsConfig$jscomp$inline_1058 = {
throw Error("TestRenderer does not support findFiberByHostInstance()");
},
bundleType: 0,
version: "18.3.0-canary-cb076b593-20240319",
version: "18.3.0-canary-83409a1fd-20240319",
rendererPackageName: "react-test-renderer"
};
var internals$jscomp$inline_1235 = {
Expand Down Expand Up @@ -9634,7 +9634,7 @@ var internals$jscomp$inline_1235 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-canary-cb076b593-20240319"
reconcilerVersion: "18.3.0-canary-83409a1fd-20240319"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_1236 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactCurrentDispatcher.current.useTransition();
};
exports.version = "18.3.0-canary-cb076b593-20240319";
exports.version = "18.3.0-canary-83409a1fd-20240319";
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
"function" ===
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
cb076b593cec3a92338958f58468cce19cb8f0d9
83409a1fdd14b2e5b33c587935a7ef552607780f

0 comments on commit 8810d83

Please sign in to comment.