From cc0b6cd97abe30bde33950e3423e0cb29bebcbf6 Mon Sep 17 00:00:00 2001 From: sebmarkbage Date: Tue, 19 Mar 2024 21:04:50 +0000 Subject: [PATCH] [Flight] Encode React Elements in Replies as Temporary References (#28564) 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
{children}
; } ``` (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
This action errored
; } return null; } ``` ``` const [errors, formAction] = useActionState(action); return
{errors}
; ``` 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 [83409a1fdd14b2e5b33c587935a7ef552607780f](https://github.com/facebook/react/commit/83409a1fdd14b2e5b33c587935a7ef552607780f) --- compiled/facebook-www/REVISION | 2 +- .../ReactDOMTesting-prod.modern.js | 6 +-- .../ReactFlightDOMClient-dev.modern.js | 45 +++++++++++++++---- .../ReactFlightDOMClient-prod.modern.js | 15 ++++++- .../ReactFlightDOMServer-dev.modern.js | 28 +++++++++++- .../ReactFlightDOMServer-prod.modern.js | 5 ++- .../facebook-www/ReactServer-dev.modern.js | 2 +- .../facebook-www/ReactServer-prod.modern.js | 2 +- .../ReactTestRenderer-dev.modern.js | 2 +- .../__test_utils__/ReactAllWarnings.js | 2 - 10 files changed, 88 insertions(+), 21 deletions(-) diff --git a/compiled/facebook-www/REVISION b/compiled/facebook-www/REVISION index 5dd41d77e7f52..5d069a975cc05 100644 --- a/compiled/facebook-www/REVISION +++ b/compiled/facebook-www/REVISION @@ -1 +1 @@ -0aab065eb3250a9714a62dc05587cbb571da7f71 +83409a1fdd14b2e5b33c587935a7ef552607780f diff --git a/compiled/facebook-www/ReactDOMTesting-prod.modern.js b/compiled/facebook-www/ReactDOMTesting-prod.modern.js index ab2866091b06a..a87104a7e2d3a 100644 --- a/compiled/facebook-www/ReactDOMTesting-prod.modern.js +++ b/compiled/facebook-www/ReactDOMTesting-prod.modern.js @@ -17142,7 +17142,7 @@ Internals.Events = [ var devToolsConfig$jscomp$inline_1781 = { findFiberByHostInstance: getClosestInstanceFromNode, bundleType: 0, - version: "18.3.0-www-modern-aa2fdf61", + version: "18.3.0-www-modern-68b315be", rendererPackageName: "react-dom" }; var internals$jscomp$inline_2151 = { @@ -17173,7 +17173,7 @@ var internals$jscomp$inline_2151 = { scheduleRoot: null, setRefreshHandler: null, getCurrentFiber: null, - reconcilerVersion: "18.3.0-www-modern-aa2fdf61" + reconcilerVersion: "18.3.0-www-modern-68b315be" }; if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) { var hook$jscomp$inline_2152 = __REACT_DEVTOOLS_GLOBAL_HOOK__; @@ -17582,4 +17582,4 @@ exports.useFormState = function (action, initialState, permalink) { exports.useFormStatus = function () { return ReactCurrentDispatcher$2.current.useHostTransitionStatus(); }; -exports.version = "18.3.0-www-modern-aa2fdf61"; +exports.version = "18.3.0-www-modern-68b315be"; diff --git a/compiled/facebook-www/ReactFlightDOMClient-dev.modern.js b/compiled/facebook-www/ReactFlightDOMClient-dev.modern.js index d944f31a052d1..57ad8a1ba738c 100644 --- a/compiled/facebook-www/ReactFlightDOMClient-dev.modern.js +++ b/compiled/facebook-www/ReactFlightDOMClient-dev.modern.js @@ -257,6 +257,17 @@ if (__DEV__) { var REACT_ELEMENT_TYPE = Symbol.for("react.element"); var REACT_LAZY_TYPE = Symbol.for("react.lazy"); + function readTemporaryReference(set, id) { + if (id < 0 || id >= set.length) { + throw new Error( + "The RSC response contained a reference that doesn't exist in the temporary reference set. " + + "Always pass the matching set that was used to create the reply when parsing its response." + ); + } + + return set[id]; + } + var knownServerReferences = new WeakMap(); // Serializable values function registerServerReference(proxy, reference, encodeFormAction) { @@ -784,19 +795,35 @@ if (__DEV__) { return createServerReferenceProxy(response, metadata); } + case "T": { + // Temporary Reference + var _id3 = parseInt(value.slice(2), 16); + + var temporaryReferences = response._tempRefs; + + if (temporaryReferences == null) { + throw new Error( + "Missing a temporary reference set but the RSC response returned a temporary reference. " + + "Pass a temporaryReference option with the set that was used with the reply." + ); + } + + return readTemporaryReference(temporaryReferences, _id3); + } + case "Q": { // Map - var _id3 = parseInt(value.slice(2), 16); + var _id4 = parseInt(value.slice(2), 16); - var data = getOutlinedModel(response, _id3); + var data = getOutlinedModel(response, _id4); return new Map(data); } case "W": { // Set - var _id4 = parseInt(value.slice(2), 16); + var _id5 = parseInt(value.slice(2), 16); - var _data = getOutlinedModel(response, _id4); + var _data = getOutlinedModel(response, _id5); return new Set(_data); } @@ -853,9 +880,9 @@ if (__DEV__) { default: { // We assume that anything else is a reference ID. - var _id5 = parseInt(value.slice(1), 16); + var _id6 = parseInt(value.slice(1), 16); - var _chunk2 = getChunk(response, _id5); + var _chunk2 = getChunk(response, _id6); switch (_chunk2.status) { case RESOLVED_MODEL: @@ -950,7 +977,8 @@ if (__DEV__) { moduleLoading, callServer, encodeFormAction, - nonce + nonce, + temporaryReferences ) { var chunks = new Map(); var response = { @@ -966,7 +994,8 @@ if (__DEV__) { _rowID: 0, _rowTag: 0, _rowLength: 0, - _buffer: [] + _buffer: [], + _tempRefs: temporaryReferences }; // Don't inline this call because it causes closure to outline the call above. response._fromJSON = createFromJSONCallback(response); diff --git a/compiled/facebook-www/ReactFlightDOMClient-prod.modern.js b/compiled/facebook-www/ReactFlightDOMClient-prod.modern.js index ddc60bbf40951..0185d915e87ac 100644 --- a/compiled/facebook-www/ReactFlightDOMClient-prod.modern.js +++ b/compiled/facebook-www/ReactFlightDOMClient-prod.modern.js @@ -269,6 +269,18 @@ function parseModelString(response, parentObject, key, value) { (parentObject = getOutlinedModel(response, parentObject)), createServerReferenceProxy(response, parentObject) ); + case "T": + parentObject = parseInt(value.slice(2), 16); + response = response._tempRefs; + if (null == response) + throw Error( + "Missing a temporary reference set but the RSC response returned a temporary reference. Pass a temporaryReference option with the set that was used with the reply." + ); + if (0 > parentObject || parentObject >= response.length) + throw Error( + "The RSC response contained a reference that doesn't exist in the temporary reference set. Always pass the matching set that was used to create the reply when parsing its response." + ); + return response[parentObject]; case "Q": return ( (parentObject = parseInt(value.slice(2), 16)), @@ -570,7 +582,8 @@ exports.createFromReadableStream = function (stream, options) { _rowID: 0, _rowTag: 0, _rowLength: 0, - _buffer: [] + _buffer: [], + _tempRefs: void 0 }; options._fromJSON = createFromJSONCallback(options); startReadingFromStream(options, stream); diff --git a/compiled/facebook-www/ReactFlightDOMServer-dev.modern.js b/compiled/facebook-www/ReactFlightDOMServer-dev.modern.js index 56f3d1406345b..29cf7f9cf0fbd 100644 --- a/compiled/facebook-www/ReactFlightDOMServer-dev.modern.js +++ b/compiled/facebook-www/ReactFlightDOMServer-dev.modern.js @@ -434,6 +434,15 @@ if (__DEV__) { var supportsRequestStorage = false; var requestStorage = null; + var TEMPORARY_REFERENCE_TAG = Symbol.for("react.temporary.reference"); // eslint-disable-next-line no-unused-vars + + function isTemporaryReference(reference) { + return reference.$$typeof === TEMPORARY_REFERENCE_TAG; + } + function resolveTemporaryReferenceID(temporaryReference) { + return temporaryReference.$$id; + } + // ATTENTION // When adding new symbols to this file, // Please consider also adding to 'react-devtools-shared/src/backend/ReactSymbols' @@ -1644,7 +1653,7 @@ if (__DEV__) { } if (typeof type === "function") { - if (isClientReference(type)) { + if (isClientReference(type) || isTemporaryReference(type)) { // This is a reference to a Client Component. return renderClientElement(task, type, key, props); } // This is a Server Component. @@ -1814,6 +1823,10 @@ if (__DEV__) { return "$F" + id.toString(16); } + function serializeTemporaryReferenceID(id) { + return "$T" + id; + } + function serializeSymbolReference(name) { return "$S" + name; } @@ -1942,6 +1955,11 @@ if (__DEV__) { return serializeServerReferenceID(metadataId); } + function serializeTemporaryReference(request, temporaryReference) { + var id = resolveTemporaryReferenceID(temporaryReference); + return serializeTemporaryReferenceID(id); + } + function serializeLargeTextString(request, text) { request.pendingChunks += 2; var textId = request.nextChunkId++; @@ -2377,6 +2395,10 @@ if (__DEV__) { return serializeServerReference(request, value); } + if (isTemporaryReference(value)) { + return serializeTemporaryReference(request, value); + } + if (/^on[A-Z]/.test(parentPropertyName)) { throw new Error( "Event handlers cannot be passed to Client Component props." + @@ -2700,6 +2722,10 @@ if (__DEV__) { parentPropertyName, value ); + } + + if (isTemporaryReference(value)) { + return serializeTemporaryReference(request, value); } // Serialize the body of the function as an eval so it can be printed. // $FlowFixMe[method-unbinding] diff --git a/compiled/facebook-www/ReactFlightDOMServer-prod.modern.js b/compiled/facebook-www/ReactFlightDOMServer-prod.modern.js index d18b26a483e40..2e5bad9414133 100644 --- a/compiled/facebook-www/ReactFlightDOMServer-prod.modern.js +++ b/compiled/facebook-www/ReactFlightDOMServer-prod.modern.js @@ -164,7 +164,8 @@ function trimOptions(options) { ((hasProperties = !0), (trimmed[key] = options[key])); return hasProperties ? trimmed : null; } -var REACT_ELEMENT_TYPE = Symbol.for("react.element"), +var TEMPORARY_REFERENCE_TAG = Symbol.for("react.temporary.reference"), + REACT_ELEMENT_TYPE = Symbol.for("react.element"), REACT_FRAGMENT_TYPE = Symbol.for("react.fragment"), REACT_CONTEXT_TYPE = Symbol.for("react.context"), REACT_FORWARD_REF_TYPE = Symbol.for("react.forward_ref"), @@ -595,7 +596,7 @@ function renderElement(request, task, type, key, ref, props) { "Refs cannot be used in Server Components, nor passed to Client Components." ); if ("function" === typeof type) - return isClientReference(type) + return isClientReference(type) || type.$$typeof === TEMPORARY_REFERENCE_TAG ? renderClientElement(task, type, key, props) : renderFunctionComponent(request, task, key, type, props); if ("string" === typeof type) diff --git a/compiled/facebook-www/ReactServer-dev.modern.js b/compiled/facebook-www/ReactServer-dev.modern.js index 7a52589048d51..71d936b58748f 100644 --- a/compiled/facebook-www/ReactServer-dev.modern.js +++ b/compiled/facebook-www/ReactServer-dev.modern.js @@ -2809,7 +2809,7 @@ if (__DEV__) { console["error"](error); }; - var ReactVersion = "18.3.0-www-modern-1e9c6c79"; + var ReactVersion = "18.3.0-www-modern-9f10bc05"; // Patch fetch var Children = { diff --git a/compiled/facebook-www/ReactServer-prod.modern.js b/compiled/facebook-www/ReactServer-prod.modern.js index 15861ace98284..c60dde8dc66ed 100644 --- a/compiled/facebook-www/ReactServer-prod.modern.js +++ b/compiled/facebook-www/ReactServer-prod.modern.js @@ -524,4 +524,4 @@ exports.useId = function () { exports.useMemo = function (create, deps) { return ReactCurrentDispatcher.current.useMemo(create, deps); }; -exports.version = "18.3.0-www-modern-7ba66f9a"; +exports.version = "18.3.0-www-modern-e9e628b6"; diff --git a/compiled/facebook-www/ReactTestRenderer-dev.modern.js b/compiled/facebook-www/ReactTestRenderer-dev.modern.js index 7c65208cad540..ed7c64914f02a 100644 --- a/compiled/facebook-www/ReactTestRenderer-dev.modern.js +++ b/compiled/facebook-www/ReactTestRenderer-dev.modern.js @@ -26056,7 +26056,7 @@ if (__DEV__) { return root; } - var ReactVersion = "18.3.0-www-modern-05b3f7fc"; + var ReactVersion = "18.3.0-www-modern-34ddd034"; // Might add PROFILE later. diff --git a/compiled/facebook-www/__test_utils__/ReactAllWarnings.js b/compiled/facebook-www/__test_utils__/ReactAllWarnings.js index 567318d5ddf79..de5ed513eacaa 100644 --- a/compiled/facebook-www/__test_utils__/ReactAllWarnings.js +++ b/compiled/facebook-www/__test_utils__/ReactAllWarnings.js @@ -221,8 +221,6 @@ export default [ "Profiler must specify an \"id\" of type `string` as a prop. Received the type `%s` instead.", "Prop `%s` did not match. Server: %s Client: %s", "React Context Providers cannot be passed to Server Functions from the Client.%s", - "React Element cannot be passed to Server Functions from the Client.%s", - "React Lazy cannot be passed to Server Functions from the Client.%s", "React depends on Map and Set built-in types. Make sure that you load a polyfill in older browsers. https://react.dev/link/react-polyfills", "React does not recognize the `%s` prop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase `%s` instead. If you accidentally passed it from a parent component, remove it from the DOM element.", "React encountered a with a `precedence` prop that also included %s. The presence of loading and error handlers indicates an intent to manage the stylesheet loading state from your from your Component code and React will not hoist or deduplicate this stylesheet. If your intent was to have React hoist and deduplciate this stylesheet using the `precedence` prop remove the %s, otherwise remove the `precedence` prop.",