diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index eccd16c3171e7..a31d4a39554c3 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -43,6 +43,7 @@ import { enablePostpone, enableRefAsProp, enableFlightReadableStream, + enableOwnerStacks, } from 'shared/ReactFeatureFlags'; import { @@ -563,6 +564,7 @@ function createElement( key: mixed, props: mixed, owner: null | ReactComponentInfo, // DEV-only + stack: null | string, // DEV-only ): React$Element { let element: any; if (__DEV__ && enableRefAsProp) { @@ -623,6 +625,23 @@ function createElement( writable: true, value: null, }); + if (enableOwnerStacks) { + Object.defineProperty(element, '_debugStack', { + configurable: false, + enumerable: false, + writable: true, + value: {stack: stack}, + }); + Object.defineProperty(element, '_debugTask', { + configurable: false, + enumerable: false, + writable: true, + value: null, + }); + } + // TODO: We should be freezing the element but currently, we might write into + // _debugInfo later. We could move it into _store which remains mutable. + Object.freeze(element.props); } return element; } @@ -661,6 +680,7 @@ function createModelResolver( cyclic: boolean, response: Response, map: (response: Response, model: any) => T, + path: Array, ): (value: any) => void { let blocked; if (initializingChunkBlockedModel) { @@ -675,6 +695,9 @@ function createModelResolver( }; } return value => { + for (let i = 1; i < path.length; i++) { + value = value[path[i]]; + } parentObject[key] = map(response, value); // If this is the root object for a model reference, where `blocked.value` @@ -733,11 +756,13 @@ function createServerReferenceProxy, T>( function getOutlinedModel( response: Response, - id: number, + reference: string, parentObject: Object, key: string, map: (response: Response, model: any) => T, ): T { + const path = reference.split(':'); + const id = parseInt(path[0], 16); const chunk = getChunk(response, id); switch (chunk.status) { case RESOLVED_MODEL: @@ -750,7 +775,11 @@ function getOutlinedModel( // The status might have changed after initialization. switch (chunk.status) { case INITIALIZED: - const chunkValue = map(response, chunk.value); + let value = chunk.value; + for (let i = 1; i < path.length; i++) { + value = value[path[i]]; + } + const chunkValue = map(response, value); if (__DEV__ && chunk._debugInfo) { // If we have a direct reference to an object that was rendered by a synchronous // server component, it might have some debug info about how it was rendered. @@ -790,6 +819,7 @@ function getOutlinedModel( chunk.status === CYCLIC, response, map, + path, ), createModelReject(parentChunk), ); @@ -874,10 +904,10 @@ function parseModelString( } case 'F': { // Server Reference - const id = parseInt(value.slice(2), 16); + const ref = value.slice(2); return getOutlinedModel( response, - id, + ref, parentObject, key, createServerReferenceProxy, @@ -885,7 +915,7 @@ function parseModelString( } case 'T': { // Temporary Reference - const id = parseInt(value.slice(2), 16); + const reference = '$' + value.slice(2); const temporaryReferences = response._tempRefs; if (temporaryReferences == null) { throw new Error( @@ -893,32 +923,32 @@ function parseModelString( 'Pass a temporaryReference option with the set that was used with the reply.', ); } - return readTemporaryReference(temporaryReferences, id); + return readTemporaryReference(temporaryReferences, reference); } case 'Q': { // Map - const id = parseInt(value.slice(2), 16); - return getOutlinedModel(response, id, parentObject, key, createMap); + const ref = value.slice(2); + return getOutlinedModel(response, ref, parentObject, key, createMap); } case 'W': { // Set - const id = parseInt(value.slice(2), 16); - return getOutlinedModel(response, id, parentObject, key, createSet); + const ref = value.slice(2); + return getOutlinedModel(response, ref, parentObject, key, createSet); } case 'B': { // Blob if (enableBinaryFlight) { - const id = parseInt(value.slice(2), 16); - return getOutlinedModel(response, id, parentObject, key, createBlob); + const ref = value.slice(2); + return getOutlinedModel(response, ref, parentObject, key, createBlob); } return undefined; } case 'K': { // FormData - const id = parseInt(value.slice(2), 16); + const ref = value.slice(2); return getOutlinedModel( response, - id, + ref, parentObject, key, createFormData, @@ -926,10 +956,10 @@ function parseModelString( } case 'i': { // Iterator - const id = parseInt(value.slice(2), 16); + const ref = value.slice(2); return getOutlinedModel( response, - id, + ref, parentObject, key, extractIterator, @@ -981,8 +1011,8 @@ function parseModelString( } default: { // We assume that anything else is a reference ID. - const id = parseInt(value.slice(1), 16); - return getOutlinedModel(response, id, parentObject, key, createModel); + const ref = value.slice(1); + return getOutlinedModel(response, ref, parentObject, key, createModel); } } } @@ -1003,6 +1033,7 @@ function parseModelTuple( tuple[2], tuple[3], __DEV__ ? (tuple: any)[4] : null, + __DEV__ && enableOwnerStacks ? (tuple: any)[5] : null, ); } return value; diff --git a/packages/react-client/src/ReactFlightReplyClient.js b/packages/react-client/src/ReactFlightReplyClient.js index 2e46f8e796f55..e5f7a559e7a45 100644 --- a/packages/react-client/src/ReactFlightReplyClient.js +++ b/packages/react-client/src/ReactFlightReplyClient.js @@ -109,8 +109,8 @@ function serializeServerReferenceID(id: number): string { return '$F' + id.toString(16); } -function serializeTemporaryReferenceID(id: number): string { - return '$T' + id.toString(16); +function serializeTemporaryReferenceMarker(): string { + return '$T'; } function serializeFormDataReference(id: number): string { @@ -176,6 +176,8 @@ function escapeStringValue(value: string): string { } } +interface Reference {} + export function processReply( root: ReactServerValue, formFieldPrefix: string, @@ -186,6 +188,8 @@ export function processReply( let nextPartId = 1; let pendingParts = 0; let formData: null | FormData = null; + const writtenObjects: WeakMap = new WeakMap(); + let modelRoot: null | ReactServerValue = root; function serializeTypedArray( tag: string, @@ -401,15 +405,22 @@ export function processReply( if (typeof value === 'object') { switch ((value: any).$$typeof) { case REACT_ELEMENT_TYPE: { - if (temporaryReferences === undefined) { - throw new Error( - 'React Element cannot be passed to Server Functions from the Client without a ' + - 'temporary reference set. Pass a TemporaryReferenceSet to the options.' + - (__DEV__ ? describeObjectForErrorMessage(parent, key) : ''), - ); + if (temporaryReferences !== undefined && key.indexOf(':') === -1) { + // TODO: If the property name contains a colon, we don't dedupe. Escape instead. + const parentReference = writtenObjects.get(parent); + if (parentReference !== undefined) { + // If the parent has a reference, we can refer to this object indirectly + // through the property name inside that parent. + const reference = parentReference + ':' + key; + // Store this object so that the server can refer to it later in responses. + writeTemporaryReference(temporaryReferences, reference, value); + return serializeTemporaryReferenceMarker(); + } } - return serializeTemporaryReferenceID( - writeTemporaryReference(temporaryReferences, value), + throw new Error( + 'React Element cannot be passed to Server Functions from the Client without a ' + + 'temporary reference set. Pass a TemporaryReferenceSet to the options.' + + (__DEV__ ? describeObjectForErrorMessage(parent, key) : ''), ); } case REACT_LAZY_TYPE: { @@ -427,7 +438,7 @@ export function processReply( // We always outline this as a separate part even though we could inline it // because it ensures a more deterministic encoding. const lazyId = nextPartId++; - const partJSON = JSON.stringify(resolvedModel, resolveToJSON); + const partJSON = serializeModel(resolvedModel, lazyId); // $FlowFixMe[incompatible-type] We know it's not null because we assigned it above. const data: FormData = formData; // eslint-disable-next-line react-internal/safe-string-coercion @@ -447,7 +458,7 @@ export function processReply( // While the first promise resolved, its value isn't necessarily what we'll // resolve into because we might suspend again. try { - const partJSON = JSON.stringify(value, resolveToJSON); + const partJSON = serializeModel(value, lazyId); // $FlowFixMe[incompatible-type] We know it's not null because we assigned it above. const data: FormData = formData; // eslint-disable-next-line react-internal/safe-string-coercion @@ -488,7 +499,7 @@ export function processReply( thenable.then( partValue => { try { - const partJSON = JSON.stringify(partValue, resolveToJSON); + const partJSON = serializeModel(partValue, promiseId); // $FlowFixMe[incompatible-type] We know it's not null because we assigned it above. const data: FormData = formData; // eslint-disable-next-line react-internal/safe-string-coercion @@ -507,6 +518,33 @@ export function processReply( ); return serializePromiseID(promiseId); } + + const existingReference = writtenObjects.get(value); + if (existingReference !== undefined) { + if (modelRoot === value) { + // This is the ID we're currently emitting so we need to write it + // once but if we discover it again, we refer to it by id. + modelRoot = null; + } else { + // We've already emitted this as an outlined object, so we can + // just refer to that by its existing ID. + return existingReference; + } + } else if (key.indexOf(':') === -1) { + // TODO: If the property name contains a colon, we don't dedupe. Escape instead. + const parentReference = writtenObjects.get(parent); + if (parentReference !== undefined) { + // If the parent has a reference, we can refer to this object indirectly + // through the property name inside that parent. + const reference = parentReference + ':' + key; + writtenObjects.set(value, reference); + if (temporaryReferences !== undefined) { + // Store this object so that the server can refer to it later in responses. + writeTemporaryReference(temporaryReferences, reference, value); + } + } + } + if (isArray(value)) { // $FlowFixMe[incompatible-return] return value; @@ -530,20 +568,20 @@ export function processReply( return serializeFormDataReference(refId); } if (value instanceof Map) { - const partJSON = JSON.stringify(Array.from(value), resolveToJSON); + const mapId = nextPartId++; + const partJSON = serializeModel(Array.from(value), mapId); if (formData === null) { formData = new FormData(); } - const mapId = nextPartId++; formData.append(formFieldPrefix + mapId, partJSON); return serializeMapID(mapId); } if (value instanceof Set) { - const partJSON = JSON.stringify(Array.from(value), resolveToJSON); + const setId = nextPartId++; + const partJSON = serializeModel(Array.from(value), setId); if (formData === null) { formData = new FormData(); } - const setId = nextPartId++; formData.append(formFieldPrefix + setId, partJSON); return serializeSetID(setId); } @@ -622,14 +660,14 @@ export function processReply( const iterator = iteratorFn.call(value); if (iterator === value) { // Iterator, not Iterable - const partJSON = JSON.stringify( + const iteratorId = nextPartId++; + const partJSON = serializeModel( Array.from((iterator: any)), - resolveToJSON, + iteratorId, ); if (formData === null) { formData = new FormData(); } - const iteratorId = nextPartId++; formData.append(formFieldPrefix + iteratorId, partJSON); return serializeIteratorID(iteratorId); } @@ -667,10 +705,9 @@ export function processReply( 'Classes or null prototypes are not supported.', ); } - // We can serialize class instances as temporary references. - return serializeTemporaryReferenceID( - writeTemporaryReference(temporaryReferences, value), - ); + // We will have written this object to the temporary reference set above + // so we can replace it with a marker to refer to this slot later. + return serializeTemporaryReferenceMarker(); } if (__DEV__) { if ( @@ -751,27 +788,41 @@ export function processReply( formData.set(formFieldPrefix + refId, metaDataJSON); return serializeServerReferenceID(refId); } - if (temporaryReferences === undefined) { - throw new Error( - 'Client Functions cannot be passed directly to Server Functions. ' + - 'Only Functions passed from the Server can be passed back again.', - ); + if (temporaryReferences !== undefined && key.indexOf(':') === -1) { + // TODO: If the property name contains a colon, we don't dedupe. Escape instead. + const parentReference = writtenObjects.get(parent); + if (parentReference !== undefined) { + // If the parent has a reference, we can refer to this object indirectly + // through the property name inside that parent. + const reference = parentReference + ':' + key; + // Store this object so that the server can refer to it later in responses. + writeTemporaryReference(temporaryReferences, reference, value); + return serializeTemporaryReferenceMarker(); + } } - return serializeTemporaryReferenceID( - writeTemporaryReference(temporaryReferences, value), + throw new Error( + 'Client Functions cannot be passed directly to Server Functions. ' + + 'Only Functions passed from the Server can be passed back again.', ); } if (typeof value === 'symbol') { - if (temporaryReferences === undefined) { - throw new Error( - 'Symbols cannot be passed to a Server Function without a ' + - 'temporary reference set. Pass a TemporaryReferenceSet to the options.' + - (__DEV__ ? describeObjectForErrorMessage(parent, key) : ''), - ); + if (temporaryReferences !== undefined && key.indexOf(':') === -1) { + // TODO: If the property name contains a colon, we don't dedupe. Escape instead. + const parentReference = writtenObjects.get(parent); + if (parentReference !== undefined) { + // If the parent has a reference, we can refer to this object indirectly + // through the property name inside that parent. + const reference = parentReference + ':' + key; + // Store this object so that the server can refer to it later in responses. + writeTemporaryReference(temporaryReferences, reference, value); + return serializeTemporaryReferenceMarker(); + } } - return serializeTemporaryReferenceID( - writeTemporaryReference(temporaryReferences, value), + throw new Error( + 'Symbols cannot be passed to a Server Function without a ' + + 'temporary reference set. Pass a TemporaryReferenceSet to the options.' + + (__DEV__ ? describeObjectForErrorMessage(parent, key) : ''), ); } @@ -784,8 +835,22 @@ export function processReply( ); } - // $FlowFixMe[incompatible-type] it's not going to be undefined because we'll encode it. - const json: string = JSON.stringify(root, resolveToJSON); + function serializeModel(model: ReactServerValue, id: number): string { + if (typeof model === 'object' && model !== null) { + const reference = serializeByValueID(id); + writtenObjects.set(model, reference); + if (temporaryReferences !== undefined) { + // Store this object so that the server can refer to it later in responses. + writeTemporaryReference(temporaryReferences, reference, model); + } + } + modelRoot = model; + // $FlowFixMe[incompatible-return] it's not going to be undefined because we'll encode it. + return JSON.stringify(model, resolveToJSON); + } + + const json = serializeModel(root, 0); + if (formData === null) { // If it's a simple data structure, we just use plain JSON. resolve(json); diff --git a/packages/react-client/src/ReactFlightTemporaryReferences.js b/packages/react-client/src/ReactFlightTemporaryReferences.js index 7060562eb2291..5ad92b3a10638 100644 --- a/packages/react-client/src/ReactFlightTemporaryReferences.js +++ b/packages/react-client/src/ReactFlightTemporaryReferences.js @@ -9,33 +9,23 @@ interface Reference {} -export opaque type TemporaryReferenceSet = Array; +export opaque type TemporaryReferenceSet = Map; export function createTemporaryReferenceSet(): TemporaryReferenceSet { - return []; + return new Map(); } export function writeTemporaryReference( set: TemporaryReferenceSet, + reference: string, object: Reference | symbol, -): number { - // We always create a new entry regardless if we've already written the same - // object. This ensures that we always generate a deterministic encoding of - // each slot in the reply for cacheability. - const newId = set.length; - set.push(object); - return newId; +): void { + set.set(reference, object); } export function readTemporaryReference( set: TemporaryReferenceSet, - id: number, + reference: string, ): T { - 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]: any); + return (set.get(reference): any); } diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index e7aa52dacb4ae..b49fb79dd12b1 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -21,12 +21,24 @@ if (typeof File === 'undefined' || typeof FormData === 'undefined') { function normalizeCodeLocInfo(str) { return ( str && - str.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function (m, name) { - return '\n in ' + name + (/\d/.test(m) ? ' (at **)' : ''); + str.replace(/^ +(?:at|in) ([\S]+)[^\n]*/gm, function (m, name) { + return ' in ' + name + (/\d/.test(m) ? ' (at **)' : ''); }) ); } +function getDebugInfo(obj) { + const debugInfo = obj._debugInfo; + if (debugInfo) { + for (let i = 0; i < debugInfo.length; i++) { + if (typeof debugInfo[i].stack === 'string') { + debugInfo[i].stack = normalizeCodeLocInfo(debugInfo[i].stack); + } + } + } + return debugInfo; +} + const heldValues = []; let finalizationCallback; function FinalizationRegistryMock(callback) { @@ -221,8 +233,19 @@ describe('ReactFlight', () => { await act(async () => { const rootModel = await ReactNoopFlightClient.read(transport); const greeting = rootModel.greeting; - expect(greeting._debugInfo).toEqual( - __DEV__ ? [{name: 'Greeting', env: 'Server', owner: null}] : undefined, + expect(getDebugInfo(greeting)).toEqual( + __DEV__ + ? [ + { + name: 'Greeting', + env: 'Server', + owner: null, + stack: gate(flag => flag.enableOwnerStacks) + ? ' in Object. (at **)' + : undefined, + }, + ] + : undefined, ); ReactNoop.render(greeting); }); @@ -248,8 +271,19 @@ describe('ReactFlight', () => { await act(async () => { const promise = ReactNoopFlightClient.read(transport); - expect(promise._debugInfo).toEqual( - __DEV__ ? [{name: 'Greeting', env: 'Server', owner: null}] : undefined, + expect(getDebugInfo(promise)).toEqual( + __DEV__ + ? [ + { + name: 'Greeting', + env: 'Server', + owner: null, + stack: gate(flag => flag.enableOwnerStacks) + ? ' in Object. (at **)' + : undefined, + }, + ] + : undefined, ); ReactNoop.render(await promise); }); @@ -2233,9 +2267,11 @@ describe('ReactFlight', () => { return !; } - const lazy = React.lazy(async () => ({ - default: , - })); + const lazy = React.lazy(async function myLazy() { + return { + default: , + }; + }); function ThirdPartyComponent() { return stranger; @@ -2269,31 +2305,61 @@ describe('ReactFlight', () => { await act(async () => { const promise = ReactNoopFlightClient.read(transport); - expect(promise._debugInfo).toEqual( + expect(getDebugInfo(promise)).toEqual( __DEV__ - ? [{name: 'ServerComponent', env: 'Server', owner: null}] + ? [ + { + name: 'ServerComponent', + env: 'Server', + owner: null, + stack: gate(flag => flag.enableOwnerStacks) + ? ' in Object. (at **)' + : undefined, + }, + ] : undefined, ); const result = await promise; const thirdPartyChildren = await result.props.children[1]; // We expect the debug info to be transferred from the inner stream to the outer. - expect(thirdPartyChildren[0]._debugInfo).toEqual( + expect(getDebugInfo(thirdPartyChildren[0])).toEqual( __DEV__ - ? [{name: 'ThirdPartyComponent', env: 'third-party', owner: null}] + ? [ + { + name: 'ThirdPartyComponent', + env: 'third-party', + owner: null, + stack: gate(flag => flag.enableOwnerStacks) + ? ' in Object. (at **)' + : undefined, + }, + ] : undefined, ); - expect(thirdPartyChildren[1]._debugInfo).toEqual( + expect(getDebugInfo(thirdPartyChildren[1])).toEqual( __DEV__ - ? [{name: 'ThirdPartyLazyComponent', env: 'third-party', owner: null}] + ? [ + { + name: 'ThirdPartyLazyComponent', + env: 'third-party', + owner: null, + stack: gate(flag => flag.enableOwnerStacks) + ? ' in myLazy (at **)\n in lazyInitializer (at **)' + : undefined, + }, + ] : undefined, ); - expect(thirdPartyChildren[2]._debugInfo).toEqual( + expect(getDebugInfo(thirdPartyChildren[2])).toEqual( __DEV__ ? [ { name: 'ThirdPartyFragmentComponent', env: 'third-party', owner: null, + stack: gate(flag => flag.enableOwnerStacks) + ? ' in Object. (at **)' + : undefined, }, ] : undefined, @@ -2357,24 +2423,47 @@ describe('ReactFlight', () => { await act(async () => { const promise = ReactNoopFlightClient.read(transport); - expect(promise._debugInfo).toEqual( + expect(getDebugInfo(promise)).toEqual( __DEV__ - ? [{name: 'ServerComponent', env: 'Server', owner: null}] + ? [ + { + name: 'ServerComponent', + env: 'Server', + owner: null, + stack: gate(flag => flag.enableOwnerStacks) + ? ' in Object. (at **)' + : undefined, + }, + ] : undefined, ); const result = await promise; const thirdPartyFragment = await result.props.children; - expect(thirdPartyFragment._debugInfo).toEqual( - __DEV__ ? [{name: 'Keyed', env: 'Server', owner: null}] : undefined, + expect(getDebugInfo(thirdPartyFragment)).toEqual( + __DEV__ + ? [ + { + name: 'Keyed', + env: 'Server', + owner: null, + stack: gate(flag => flag.enableOwnerStacks) + ? ' in ServerComponent (at **)' + : undefined, + }, + ] + : undefined, ); // We expect the debug info to be transferred from the inner stream to the outer. - expect(thirdPartyFragment.props.children._debugInfo).toEqual( + expect(getDebugInfo(thirdPartyFragment.props.children)).toEqual( __DEV__ ? [ { name: 'ThirdPartyAsyncIterableComponent', env: 'third-party', owner: null, + stack: gate(flag => flag.enableOwnerStacks) + ? ' in Object. (at **)' + : undefined, }, ] : undefined, @@ -2467,10 +2556,24 @@ describe('ReactFlight', () => { // We've rendered down to the span. expect(greeting.type).toBe('span'); if (__DEV__) { - const greetInfo = {name: 'Greeting', env: 'Server', owner: null}; - expect(greeting._debugInfo).toEqual([ + const greetInfo = { + name: 'Greeting', + env: 'Server', + owner: null, + stack: gate(flag => flag.enableOwnerStacks) + ? ' in Object. (at **)' + : undefined, + }; + expect(getDebugInfo(greeting)).toEqual([ greetInfo, - {name: 'Container', env: 'Server', owner: greetInfo}, + { + name: 'Container', + env: 'Server', + owner: greetInfo, + stack: gate(flag => flag.enableOwnerStacks) + ? ' in Greeting (at **)' + : undefined, + }, ]); // The owner that created the span was the outer server component. // We expect the debug info to be referentially equal to the owner. diff --git a/packages/react-dom-bindings/src/events/plugins/FormActionEventPlugin.js b/packages/react-dom-bindings/src/events/plugins/FormActionEventPlugin.js index 8f874e2cdb8c4..496b55e15e79b 100644 --- a/packages/react-dom-bindings/src/events/plugins/FormActionEventPlugin.js +++ b/packages/react-dom-bindings/src/events/plugins/FormActionEventPlugin.js @@ -14,11 +14,61 @@ import type {EventSystemFlags} from '../EventSystemFlags'; import type {Fiber} from 'react-reconciler/src/ReactInternalTypes'; import type {FormStatus} from 'react-dom-bindings/src/shared/ReactDOMFormActions'; +import {enableTrustedTypesIntegration} from 'shared/ReactFeatureFlags'; import {getFiberCurrentPropsFromNode} from '../../client/ReactDOMComponentTree'; import {startHostTransition} from 'react-reconciler/src/ReactFiberReconciler'; +import {didCurrentEventScheduleTransition} from 'react-reconciler/src/ReactFiberRootScheduler'; +import sanitizeURL from 'react-dom-bindings/src/shared/sanitizeURL'; +import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion'; import {SyntheticEvent} from '../SyntheticEvent'; +function coerceFormActionProp( + actionProp: mixed, +): string | (FormData => void | Promise) | null { + // This should match the logic in ReactDOMComponent + if ( + actionProp == null || + typeof actionProp === 'symbol' || + typeof actionProp === 'boolean' + ) { + return null; + } else if (typeof actionProp === 'function') { + return (actionProp: any); + } else { + if (__DEV__) { + checkAttributeStringCoercion(actionProp, 'action'); + } + return (sanitizeURL( + enableTrustedTypesIntegration ? actionProp : '' + (actionProp: any), + ): any); + } +} + +function createFormDataWithSubmitter( + form: HTMLFormElement, + submitter: HTMLInputElement | HTMLButtonElement, +) { + // The submitter's value should be included in the FormData. + // It should be in the document order in the form. + // Since the FormData constructor invokes the formdata event it also + // needs to be available before that happens so after construction it's too + // late. We use a temporary fake node for the duration of this event. + // TODO: FormData takes a second argument that it's the submitter but this + // is fairly new so not all browsers support it yet. Switch to that technique + // when available. + const temp = submitter.ownerDocument.createElement('input'); + temp.name = submitter.name; + temp.value = submitter.value; + if (form.id) { + temp.setAttribute('form', form.id); + } + (submitter.parentNode: any).insertBefore(temp, submitter); + const formData = new FormData(form); + (temp.parentNode: any).removeChild(temp); + return formData; +} + /** * This plugin invokes action functions on forms, inputs and buttons if * the form doesn't prevent default. @@ -42,16 +92,19 @@ function extractEvents( } const formInst = maybeTargetInst; const form: HTMLFormElement = (nativeEventTarget: any); - let action = (getFiberCurrentPropsFromNode(form): any).action; - let submitter: null | HTMLInputElement | HTMLButtonElement = + let action = coerceFormActionProp( + (getFiberCurrentPropsFromNode(form): any).action, + ); + let submitter: null | void | HTMLInputElement | HTMLButtonElement = (nativeEvent: any).submitter; let submitterAction; if (submitter) { const submitterProps = getFiberCurrentPropsFromNode(submitter); submitterAction = submitterProps - ? (submitterProps: any).formAction - : submitter.getAttribute('formAction'); - if (submitterAction != null) { + ? coerceFormActionProp((submitterProps: any).formAction) + : // The built-in Flow type is ?string, wider than the spec + ((submitter.getAttribute('formAction'): any): string | null); + if (submitterAction !== null) { // The submitter overrides the form action. action = submitterAction; // If the action is a function, we don't want to pass its name @@ -60,10 +113,6 @@ function extractEvents( } } - if (typeof action !== 'function') { - return; - } - const event = new SyntheticEvent( 'action', 'action', @@ -74,44 +123,60 @@ function extractEvents( function submitForm() { if (nativeEvent.defaultPrevented) { - // We let earlier events to prevent the action from submitting. - return; - } - // Prevent native navigation. - event.preventDefault(); - let formData; - if (submitter) { - // The submitter's value should be included in the FormData. - // It should be in the document order in the form. - // Since the FormData constructor invokes the formdata event it also - // needs to be available before that happens so after construction it's too - // late. We use a temporary fake node for the duration of this event. - // TODO: FormData takes a second argument that it's the submitter but this - // is fairly new so not all browsers support it yet. Switch to that technique - // when available. - const temp = submitter.ownerDocument.createElement('input'); - temp.name = submitter.name; - temp.value = submitter.value; - if (form.id) { - temp.setAttribute('form', form.id); + // An earlier event prevented form submission. If a transition update was + // also scheduled, we should trigger a pending form status — even if + // no action function was provided. + if (didCurrentEventScheduleTransition()) { + // We're going to set the pending form status, but because the submission + // was prevented, we should not fire the action function. + const formData = submitter + ? createFormDataWithSubmitter(form, submitter) + : new FormData(form); + const pendingState: FormStatus = { + pending: true, + data: formData, + method: form.method, + action: action, + }; + if (__DEV__) { + Object.freeze(pendingState); + } + startHostTransition( + formInst, + pendingState, + // Pass `null` as the action + // TODO: Consider splitting up startHostTransition into two separate + // functions, one that sets the form status and one that invokes + // the action. + null, + formData, + ); + } else { + // No earlier event scheduled a transition. Exit without setting a + // pending form status. } - (submitter.parentNode: any).insertBefore(temp, submitter); - formData = new FormData(form); - (temp.parentNode: any).removeChild(temp); - } else { - formData = new FormData(form); - } + } else if (typeof action === 'function') { + // A form action was provided. Prevent native navigation. + event.preventDefault(); - const pendingState: FormStatus = { - pending: true, - data: formData, - method: form.method, - action: action, - }; - if (__DEV__) { - Object.freeze(pendingState); + // Dispatch the action and set a pending form status. + const formData = submitter + ? createFormDataWithSubmitter(form, submitter) + : new FormData(form); + const pendingState: FormStatus = { + pending: true, + data: formData, + method: form.method, + action: action, + }; + if (__DEV__) { + Object.freeze(pendingState); + } + startHostTransition(formInst, pendingState, action, formData); + } else { + // No earlier event prevented the default submission, and no action was + // provided. Exit without setting a pending form status. } - startHostTransition(formInst, pendingState, action, formData); } dispatchQueue.push({ diff --git a/packages/react-dom-bindings/src/shared/ReactDOMFormActions.js b/packages/react-dom-bindings/src/shared/ReactDOMFormActions.js index 0b3f478e33b19..ab36fc11199b1 100644 --- a/packages/react-dom-bindings/src/shared/ReactDOMFormActions.js +++ b/packages/react-dom-bindings/src/shared/ReactDOMFormActions.js @@ -25,7 +25,7 @@ type FormStatusPending = {| pending: true, data: FormData, method: string, - action: string | (FormData => void | Promise), + action: string | (FormData => void | Promise) | null, |}; export type FormStatus = FormStatusPending | FormStatusNotPending; diff --git a/packages/react-dom/src/__tests__/ReactDOMForm-test.js b/packages/react-dom/src/__tests__/ReactDOMForm-test.js index 5db3b2fd8a5be..f4dda30c45143 100644 --- a/packages/react-dom/src/__tests__/ReactDOMForm-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMForm-test.js @@ -35,10 +35,12 @@ describe('ReactDOMForm', () => { let ReactDOMClient; let Scheduler; let assertLog; + let assertConsoleErrorDev; let waitForThrow; let useState; let Suspense; let startTransition; + let useTransition; let use; let textCache; let useFormStatus; @@ -54,9 +56,12 @@ describe('ReactDOMForm', () => { act = require('internal-test-utils').act; assertLog = require('internal-test-utils').assertLog; waitForThrow = require('internal-test-utils').waitForThrow; + assertConsoleErrorDev = + require('internal-test-utils').assertConsoleErrorDev; useState = React.useState; Suspense = React.Suspense; startTransition = React.startTransition; + useTransition = React.useTransition; use = React.use; useFormStatus = ReactDOM.useFormStatus; requestFormReset = ReactDOM.requestFormReset; @@ -1782,4 +1787,306 @@ describe('ReactDOMForm', () => { // The form was reset even though the action didn't finish. expect(inputRef.current.value).toBe('Initial'); }); + + test("regression: submitter's formAction prop is coerced correctly before checking if it exists", async () => { + function App({submitterAction}) { + return ( +
Scheduler.log('Form action')}> +