From 8e76371241646038a4c98426d8da59fe263538d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 29 Mar 2021 22:35:34 -0400 Subject: [PATCH 01/18] Move not shared to client (#21135) --- .../src/{shared => client}/CSSPropertyOperations.js | 6 +++--- .../src/{shared => client}/CSSShorthandProperty.js | 0 packages/react-dom/src/client/ReactDOMComponent.js | 2 +- .../createMicrosoftUnsafeLocalFunction.js | 0 packages/react-dom/src/client/setInnerHTML.js | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) rename packages/react-dom/src/{shared => client}/CSSPropertyOperations.js (96%) rename packages/react-dom/src/{shared => client}/CSSShorthandProperty.js (100%) rename packages/react-dom/src/{shared => client}/createMicrosoftUnsafeLocalFunction.js (100%) diff --git a/packages/react-dom/src/shared/CSSPropertyOperations.js b/packages/react-dom/src/client/CSSPropertyOperations.js similarity index 96% rename from packages/react-dom/src/shared/CSSPropertyOperations.js rename to packages/react-dom/src/client/CSSPropertyOperations.js index 8aaa78fe68724..12c85b4929296 100644 --- a/packages/react-dom/src/shared/CSSPropertyOperations.js +++ b/packages/react-dom/src/client/CSSPropertyOperations.js @@ -7,9 +7,9 @@ import {shorthandToLonghand} from './CSSShorthandProperty'; -import dangerousStyleValue from './dangerousStyleValue'; -import hyphenateStyleName from './hyphenateStyleName'; -import warnValidStyle from './warnValidStyle'; +import dangerousStyleValue from '../shared/dangerousStyleValue'; +import hyphenateStyleName from '../shared/hyphenateStyleName'; +import warnValidStyle from '../shared/warnValidStyle'; /** * Operations for dealing with CSS properties. diff --git a/packages/react-dom/src/shared/CSSShorthandProperty.js b/packages/react-dom/src/client/CSSShorthandProperty.js similarity index 100% rename from packages/react-dom/src/shared/CSSShorthandProperty.js rename to packages/react-dom/src/client/CSSShorthandProperty.js diff --git a/packages/react-dom/src/client/ReactDOMComponent.js b/packages/react-dom/src/client/ReactDOMComponent.js index 71da034466c8a..0356f6dc8ff44 100644 --- a/packages/react-dom/src/client/ReactDOMComponent.js +++ b/packages/react-dom/src/client/ReactDOMComponent.js @@ -53,7 +53,7 @@ import { createDangerousStringForStyles, setValueForStyles, validateShorthandPropertyCollisionInDev, -} from '../shared/CSSPropertyOperations'; +} from './CSSPropertyOperations'; import {HTML_NAMESPACE, getIntrinsicNamespace} from '../shared/DOMNamespaces'; import { getPropertyInfo, diff --git a/packages/react-dom/src/shared/createMicrosoftUnsafeLocalFunction.js b/packages/react-dom/src/client/createMicrosoftUnsafeLocalFunction.js similarity index 100% rename from packages/react-dom/src/shared/createMicrosoftUnsafeLocalFunction.js rename to packages/react-dom/src/client/createMicrosoftUnsafeLocalFunction.js diff --git a/packages/react-dom/src/client/setInnerHTML.js b/packages/react-dom/src/client/setInnerHTML.js index 43319acd2bf03..e283efb270770 100644 --- a/packages/react-dom/src/client/setInnerHTML.js +++ b/packages/react-dom/src/client/setInnerHTML.js @@ -8,7 +8,7 @@ */ import {SVG_NAMESPACE} from '../shared/DOMNamespaces'; -import createMicrosoftUnsafeLocalFunction from '../shared/createMicrosoftUnsafeLocalFunction'; +import createMicrosoftUnsafeLocalFunction from './createMicrosoftUnsafeLocalFunction'; import {enableTrustedTypesIntegration} from 'shared/ReactFeatureFlags'; // SVG temp container for IE lacking innerHTML From cecfde51b63ff33afde0d63ed32de53782c6e14c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 29 Mar 2021 22:35:47 -0400 Subject: [PATCH 02/18] Don't import star from ReactDOM (#21133) --- packages/react-dom/src/client/ReactDOMOption.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMOption.js b/packages/react-dom/src/client/ReactDOMOption.js index 4dfe661b23f15..56a4efe631519 100644 --- a/packages/react-dom/src/client/ReactDOMOption.js +++ b/packages/react-dom/src/client/ReactDOMOption.js @@ -7,7 +7,7 @@ * @flow */ -import * as React from 'react'; +import {Children} from 'react'; import {getToStringValue, toString} from './ToStringValue'; let didWarnSelectedSetOnOption = false; @@ -21,7 +21,7 @@ function flattenChildren(children) { // Note that this would throw on non-element objects. // Elements are stringified (which is normally irrelevant // but matters for ). - React.Children.forEach(children, function(child) { + Children.forEach(children, function(child) { if (child == null) { return; } @@ -45,7 +45,7 @@ export function validateProps(element: Element, props: Object) { // TODO: this seems like it could cause a DEV-only throw for hydration // if children contains a non-element object. We should try to avoid that. if (typeof props.children === 'object' && props.children !== null) { - React.Children.forEach(props.children, function(child) { + Children.forEach(props.children, function(child) { if (child == null) { return; } From e40f0b260303441710d762a83afc8c41aefe1daa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 29 Mar 2021 22:35:58 -0400 Subject: [PATCH 03/18] Remove checkReact (#21132) I don't know what this is useful for but I suspect it was only useful at FB and is not applicable to ES modules at FB nor elsewhere. --- packages/react-dom/src/client/ReactDOM.js | 1 - packages/react-dom/src/shared/checkReact.js | 17 ----------------- 2 files changed, 18 deletions(-) delete mode 100644 packages/react-dom/src/shared/checkReact.js diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index aeeb5b3070639..9223c52fb027c 100644 --- a/packages/react-dom/src/client/ReactDOM.js +++ b/packages/react-dom/src/client/ReactDOM.js @@ -10,7 +10,6 @@ import type {ReactNodeList} from 'shared/ReactTypes'; import type {Container} from './ReactDOMHostConfig'; -import '../shared/checkReact'; import { findDOMNode, render, diff --git a/packages/react-dom/src/shared/checkReact.js b/packages/react-dom/src/shared/checkReact.js deleted file mode 100644 index 860628b81fb71..0000000000000 --- a/packages/react-dom/src/shared/checkReact.js +++ /dev/null @@ -1,17 +0,0 @@ -/** - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow - */ - -import * as React from 'react'; -import invariant from 'shared/invariant'; - -invariant( - React, - 'ReactDOM was loaded before React. Make sure you load ' + - 'the React package before loading ReactDOM.', -); From d1294c9d404de748b49dd6543144367f90a5e6b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 29 Mar 2021 22:36:16 -0400 Subject: [PATCH 04/18] [Flight] Add global onError handler (#21129) * Add onError option to Flight Server The callback is called any time an error is generated in a server component. This allows it to be logged on a server if needed. It'll still be rethrown on the client so it can be logged there too but in case it never reaches the client, here's a way to make sure it doesn't get lost. * Add fatal error handling --- .../src/ReactNoopFlightServer.js | 7 ++- .../src/ReactFlightDOMRelayServer.js | 12 ++++- .../ReactFlightDOMRelayServerHostConfig.js | 7 ++- .../src/ReactFlightDOMServerBrowser.js | 12 ++++- .../src/ReactFlightDOMServerNode.js | 12 ++++- .../src/__tests__/ReactFlightDOM-test.js | 17 ++++++- .../ReactFlightNativeRelayServerHostConfig.js | 7 ++- .../react-server/src/ReactFlightServer.js | 51 ++++++++++++++----- .../src/ReactFlightServerConfigStream.js | 1 + 9 files changed, 106 insertions(+), 20 deletions(-) diff --git a/packages/react-noop-renderer/src/ReactNoopFlightServer.js b/packages/react-noop-renderer/src/ReactNoopFlightServer.js index 7614ba3e8dfbc..51a5604bd5554 100644 --- a/packages/react-noop-renderer/src/ReactNoopFlightServer.js +++ b/packages/react-noop-renderer/src/ReactNoopFlightServer.js @@ -54,13 +54,18 @@ const ReactNoopFlightServer = ReactFlightServer({ }, }); -function render(model: ReactModel): Destination { +type Options = { + onError?: (error: mixed) => void, +}; + +function render(model: ReactModel, options?: Options): Destination { const destination: Destination = []; const bundlerConfig = undefined; const request = ReactNoopFlightServer.createRequest( model, destination, bundlerConfig, + options ? options.onError : undefined, ); ReactNoopFlightServer.startWork(request); return destination; diff --git a/packages/react-server-dom-relay/src/ReactFlightDOMRelayServer.js b/packages/react-server-dom-relay/src/ReactFlightDOMRelayServer.js index 3334d34d453da..56769e4255394 100644 --- a/packages/react-server-dom-relay/src/ReactFlightDOMRelayServer.js +++ b/packages/react-server-dom-relay/src/ReactFlightDOMRelayServer.js @@ -15,12 +15,22 @@ import type { import {createRequest, startWork} from 'react-server/src/ReactFlightServer'; +type Options = { + onError?: (error: mixed) => void, +}; + function render( model: ReactModel, destination: Destination, config: BundlerConfig, + options?: Options, ): void { - const request = createRequest(model, destination, config); + const request = createRequest( + model, + destination, + config, + options ? options.onError : undefined, + ); startWork(request); } diff --git a/packages/react-server-dom-relay/src/ReactFlightDOMRelayServerHostConfig.js b/packages/react-server-dom-relay/src/ReactFlightDOMRelayServerHostConfig.js index 1aa02e00d54b5..7ebcaf6b1e8d5 100644 --- a/packages/react-server-dom-relay/src/ReactFlightDOMRelayServerHostConfig.js +++ b/packages/react-server-dom-relay/src/ReactFlightDOMRelayServerHostConfig.js @@ -26,6 +26,7 @@ import {resolveModelToJSON} from 'react-server/src/ReactFlightServer'; import { emitRow, resolveModuleMetaData as resolveModuleMetaDataImpl, + close, } from 'ReactFlightDOMRelayServerIntegration'; export type { @@ -146,4 +147,8 @@ export function writeChunk(destination: Destination, chunk: Chunk): boolean { export function completeWriting(destination: Destination) {} -export {close} from 'ReactFlightDOMRelayServerIntegration'; +export {close}; + +export function closeWithError(destination: Destination, error: mixed): void { + close(destination); +} diff --git a/packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js b/packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js index bbbf5f18ea867..accd749a56d54 100644 --- a/packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js +++ b/packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js @@ -16,14 +16,24 @@ import { startFlowing, } from 'react-server/src/ReactFlightServer'; +type Options = { + onError?: (error: mixed) => void, +}; + function renderToReadableStream( model: ReactModel, webpackMap: BundlerConfig, + options?: Options, ): ReadableStream { let request; return new ReadableStream({ start(controller) { - request = createRequest(model, controller, webpackMap); + request = createRequest( + model, + controller, + webpackMap, + options ? options.onError : undefined, + ); startWork(request); }, pull(controller) { diff --git a/packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js b/packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js index 312c18b094846..dc78c503aaf39 100644 --- a/packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js @@ -21,12 +21,22 @@ function createDrainHandler(destination, request) { return () => startFlowing(request); } +type Options = { + onError?: (error: mixed) => void, +}; + function pipeToNodeWritable( model: ReactModel, destination: Writable, webpackMap: BundlerConfig, + options?: Options, ): void { - const request = createRequest(model, destination, webpackMap); + const request = createRequest( + model, + destination, + webpackMap, + options ? options.onError : undefined, + ); destination.on('drain', createDrainHandler(destination, request)); startWork(request); } diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js index a481d6ebbcd62..a768b0cf0bb64 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js @@ -256,6 +256,7 @@ describe('ReactFlightDOM', () => { // @gate experimental it('should progressively reveal server components', async () => { + let reportedErrors = []; const {Suspense} = React; // Client Components @@ -374,7 +375,11 @@ describe('ReactFlightDOM', () => { } const {writable, readable} = getTestStream(); - ReactServerDOMWriter.pipeToNodeWritable(model, writable, webpackMap); + ReactServerDOMWriter.pipeToNodeWritable(model, writable, webpackMap, { + onError(x) { + reportedErrors.push(x); + }, + }); const response = ReactServerDOMReader.createFromReadableStream(readable); const container = document.createElement('div'); @@ -407,9 +412,12 @@ describe('ReactFlightDOM', () => { '

(loading games)

', ); + expect(reportedErrors).toEqual([]); + + const theError = new Error('Game over'); // Let's *fail* loading games. await act(async () => { - rejectGames(new Error('Game over')); + rejectGames(theError); }); expect(container.innerHTML).toBe( '
:name::avatar:
' + @@ -418,6 +426,9 @@ describe('ReactFlightDOM', () => { '

Game over

', // TODO: should not have message in prod. ); + expect(reportedErrors).toEqual([theError]); + reportedErrors = []; + // We can now show the sidebar. await act(async () => { resolvePhotos(); @@ -439,6 +450,8 @@ describe('ReactFlightDOM', () => { '
:posts:
' + '

Game over

', // TODO: should not have message in prod. ); + + expect(reportedErrors).toEqual([]); }); // @gate experimental diff --git a/packages/react-server-native-relay/src/ReactFlightNativeRelayServerHostConfig.js b/packages/react-server-native-relay/src/ReactFlightNativeRelayServerHostConfig.js index ecea013654f39..8bf0cdf8b41ef 100644 --- a/packages/react-server-native-relay/src/ReactFlightNativeRelayServerHostConfig.js +++ b/packages/react-server-native-relay/src/ReactFlightNativeRelayServerHostConfig.js @@ -25,6 +25,7 @@ import {resolveModelToJSON} from 'react-server/src/ReactFlightServer'; import { emitRow, + close, resolveModuleMetaData as resolveModuleMetaDataImpl, } from 'ReactFlightNativeRelayServerIntegration'; @@ -146,4 +147,8 @@ export function writeChunk(destination: Destination, chunk: Chunk): boolean { export function completeWriting(destination: Destination) {} -export {close} from 'ReactFlightNativeRelayServerIntegration'; +export {close}; + +export function closeWithError(destination: Destination, error: mixed): void { + close(destination); +} diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index b80798cd66f4b..2a09e21c89a00 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -24,6 +24,7 @@ import { completeWriting, flushBuffered, close, + closeWithError, processModelChunk, processModuleChunk, processSymbolChunk, @@ -83,16 +84,20 @@ export type Request = { completedErrorChunks: Array, writtenSymbols: Map, writtenModules: Map, + onError: (error: mixed) => void, flowing: boolean, toJSON: (key: string, value: ReactModel) => ReactJSONValue, }; const ReactCurrentDispatcher = ReactSharedInternals.ReactCurrentDispatcher; +function defaultErrorHandler() {} + export function createRequest( model: ReactModel, destination: Destination, bundlerConfig: BundlerConfig, + onError: (error: mixed) => void = defaultErrorHandler, ): Request { const pingedSegments = []; const request = { @@ -107,6 +112,7 @@ export function createRequest( completedErrorChunks: [], writtenSymbols: new Map(), writtenModules: new Map(), + onError, flowing: false, toJSON: function(key: string, value: ReactModel): ReactJSONValue { return resolveModelToJSON(request, this, key, value); @@ -413,6 +419,7 @@ export function resolveModelToJSON( x.then(ping, ping); return serializeByRefID(newSegment.id); } else { + reportError(request, x); // Something errored. We'll still send everything we have up until this point. // We'll replace this element with a lazy reference that throws on the client // once it gets rendered. @@ -589,6 +596,15 @@ export function resolveModelToJSON( ); } +function reportError(request: Request, error: mixed): void { + request.onError(error); +} + +function fatalError(request: Request, error: mixed): void { + // This is called outside error handling code such as if an error happens in React internals. + closeWithError(request.destination, error); +} + function emitErrorChunk(request: Request, id: number, error: mixed): void { // TODO: We should not leak error messages to the client in prod. // Give this an error code instead and log on the server. @@ -654,6 +670,7 @@ function retrySegment(request: Request, segment: Segment): void { x.then(ping, ping); return; } else { + reportError(request, x); // This errored, we need to serialize this error to the emitErrorChunk(request, segment.id, x); } @@ -666,18 +683,23 @@ function performWork(request: Request): void { ReactCurrentDispatcher.current = Dispatcher; currentCache = request.cache; - const pingedSegments = request.pingedSegments; - request.pingedSegments = []; - for (let i = 0; i < pingedSegments.length; i++) { - const segment = pingedSegments[i]; - retrySegment(request, segment); - } - if (request.flowing) { - flushCompletedChunks(request); + try { + const pingedSegments = request.pingedSegments; + request.pingedSegments = []; + for (let i = 0; i < pingedSegments.length; i++) { + const segment = pingedSegments[i]; + retrySegment(request, segment); + } + if (request.flowing) { + flushCompletedChunks(request); + } + } catch (error) { + reportError(request, error); + fatalError(request, error); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; + currentCache = prevCache; } - - ReactCurrentDispatcher.current = prevDispatcher; - currentCache = prevCache; } let reentrant = false; @@ -749,7 +771,12 @@ export function startWork(request: Request): void { export function startFlowing(request: Request): void { request.flowing = true; - flushCompletedChunks(request); + try { + flushCompletedChunks(request); + } catch (error) { + reportError(request, error); + fatalError(request, error); + } } function unsupportedHook(): void { diff --git a/packages/react-server/src/ReactFlightServerConfigStream.js b/packages/react-server/src/ReactFlightServerConfigStream.js index b9a57c16eb277..4ff841bfc11ba 100644 --- a/packages/react-server/src/ReactFlightServerConfigStream.js +++ b/packages/react-server/src/ReactFlightServerConfigStream.js @@ -126,4 +126,5 @@ export { writeChunk, completeWriting, close, + closeWithError, } from './ReactServerStreamConfig'; From 0853aab74dc8285401e62d1e490551ba3c96fd64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 29 Mar 2021 22:39:55 -0400 Subject: [PATCH 05/18] Log all errors to console.error by default (#21130) --- .../src/__tests__/ReactFlight-test.js | 19 ++++++++++++++----- packages/react-server/src/ReactFizzServer.js | 6 +++++- .../react-server/src/ReactFlightServer.js | 4 +++- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index e0947c68811d2..4961cbcff72a0 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -156,10 +156,15 @@ describe('ReactFlight', () => { return
; } - const event = ReactNoopFlightServer.render(); - const fn = ReactNoopFlightServer.render(); - const symbol = ReactNoopFlightServer.render(); - const refs = ReactNoopFlightServer.render(); + const options = { + onError() { + // ignore + }, + }; + const event = ReactNoopFlightServer.render(, options); + const fn = ReactNoopFlightServer.render(, options); + const symbol = ReactNoopFlightServer.render(, options); + const refs = ReactNoopFlightServer.render(, options); function Client({transport}) { return ReactNoopFlightClient.read(transport); @@ -213,7 +218,11 @@ describe('ReactFlight', () => { ); } - const data = ReactNoopFlightServer.render(); + const data = ReactNoopFlightServer.render(, { + onError(x) { + // ignore + }, + }); function Client({transport}) { return ReactNoopFlightClient.read(transport); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 739ba0c436aa6..27ff2814ac105 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -142,13 +142,17 @@ type Request = { // 500 * 1024 / 8 * .8 * 0.5 / 2 const DEFAULT_PROGRESSIVE_CHUNK_SIZE = 12800; +function defaultErrorHandler(error: mixed) { + console['error'](error); // Don't transform to our wrapper +} + export function createRequest( children: ReactNodeList, destination: Destination, responseState: ResponseState, rootContext: FormatContext, progressiveChunkSize: number = DEFAULT_PROGRESSIVE_CHUNK_SIZE, - onError: (error: mixed) => void = noop, + onError: (error: mixed) => void = defaultErrorHandler, onCompleteAll: () => void = noop, onReadyToStream: () => void = noop, ): Request { diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 2a09e21c89a00..d4fa3bd0623cd 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -91,7 +91,9 @@ export type Request = { const ReactCurrentDispatcher = ReactSharedInternals.ReactCurrentDispatcher; -function defaultErrorHandler() {} +function defaultErrorHandler(error: mixed) { + console['error'](error); // Don't transform to our wrapper +} export function createRequest( model: ReactModel, From 516b76b9ae177ecc38cc64ba33ddfc7cfe5f5a99 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 30 Mar 2021 16:08:50 +0100 Subject: [PATCH 06/18] [Fast Refresh] Support callthrough HOCs (#21104) * [Fast Refresh] Support callthrough HOCs * Add a newly failing testing to demonstrate the flaw This shows why my initial approach doesn't make sense. * Attach signatures at every nesting level * Sign nested memo/forwardRef too * Add an IIFE test This is not a case that is important for Fast Refresh, but we shouldn't change the code semantics. This case shows the transform isn't quite correct. It's wrapping the call at the wrong place. * Find HOCs above more precisely This fixes a false positive that was causing an IIFE to be wrapped in the wrong place, which made the wrapping unsafe. * Be defensive against non-components being passed to setSignature * Fix lint --- .../src/ReactFreshBabelPlugin.js | 49 +++- .../react-refresh/src/ReactFreshRuntime.js | 86 ++++--- .../__tests__/ReactFreshBabelPlugin-test.js | 12 + .../__tests__/ReactFreshIntegration-test.js | 221 ++++++++++++++++++ .../ReactFreshBabelPlugin-test.js.snap | 26 ++- 5 files changed, 345 insertions(+), 49 deletions(-) diff --git a/packages/react-refresh/src/ReactFreshBabelPlugin.js b/packages/react-refresh/src/ReactFreshBabelPlugin.js index 4171893e41ddd..64013db181a3f 100644 --- a/packages/react-refresh/src/ReactFreshBabelPlugin.js +++ b/packages/react-refresh/src/ReactFreshBabelPlugin.js @@ -333,6 +333,38 @@ export default function(babel, opts = {}) { return args; } + function findHOCCallPathsAbove(path) { + const calls = []; + while (true) { + if (!path) { + return calls; + } + const parentPath = path.parentPath; + if (!parentPath) { + return calls; + } + if ( + // hoc(_c = function() { }) + parentPath.node.type === 'AssignmentExpression' && + path.node === parentPath.node.right + ) { + // Ignore registrations. + path = parentPath; + continue; + } + if ( + // hoc1(hoc2(...)) + parentPath.node.type === 'CallExpression' && + path.node !== parentPath.node.callee + ) { + calls.push(parentPath); + path = parentPath; + continue; + } + return calls; // Stop at other types. + } + } + const seenForRegistration = new WeakSet(); const seenForSignature = new WeakSet(); const seenForOutro = new WeakSet(); @@ -630,13 +662,16 @@ export default function(babel, opts = {}) { // Result: let Foo = () => {}; __signature(Foo, ...); } else { // let Foo = hoc(() => {}) - path.replaceWith( - t.callExpression( - sigCallID, - createArgumentsForSignature(node, signature, path.scope), - ), - ); - // Result: let Foo = hoc(__signature(() => {}, ...)) + const paths = [path, ...findHOCCallPathsAbove(path)]; + paths.forEach(p => { + p.replaceWith( + t.callExpression( + sigCallID, + createArgumentsForSignature(p.node, signature, p.scope), + ), + ); + }); + // Result: let Foo = __signature(hoc(__signature(() => {}, ...)), ...) } }, }, diff --git a/packages/react-refresh/src/ReactFreshRuntime.js b/packages/react-refresh/src/ReactFreshRuntime.js index 669eb7c1520f0..a09222a076303 100644 --- a/packages/react-refresh/src/ReactFreshRuntime.js +++ b/packages/react-refresh/src/ReactFreshRuntime.js @@ -355,12 +355,25 @@ export function setSignature( getCustomHooks?: () => Array, ): void { if (__DEV__) { - allSignaturesByType.set(type, { - forceReset, - ownKey: key, - fullKey: null, - getCustomHooks: getCustomHooks || (() => []), - }); + if (!allSignaturesByType.has(type)) { + allSignaturesByType.set(type, { + forceReset, + ownKey: key, + fullKey: null, + getCustomHooks: getCustomHooks || (() => []), + }); + } + // Visit inner types because we might not have signed them. + if (typeof type === 'object' && type !== null) { + switch (getProperty(type, '$$typeof')) { + case REACT_FORWARD_REF_TYPE: + setSignature(type.render, key, forceReset, getCustomHooks); + break; + case REACT_MEMO_TYPE: + setSignature(type.type, key, forceReset, getCustomHooks); + break; + } + } } else { throw new Error( 'Unexpected call to React Refresh in a production environment.', @@ -609,57 +622,58 @@ export function _getMountedRootCount() { // function Hello() { // const [foo, setFoo] = useState(0); // const value = useCustomHook(); -// _s(); /* Second call triggers collecting the custom Hook list. +// _s(); /* Call without arguments triggers collecting the custom Hook list. // * This doesn't happen during the module evaluation because we // * don't want to change the module order with inline requires. // * Next calls are noops. */ // return

Hi

; // } // -// /* First call specifies the signature: */ +// /* Call with arguments attaches the signature to the type: */ // _s( // Hello, // 'useState{[foo, setFoo]}(0)', // () => [useCustomHook], /* Lazy to avoid triggering inline requires */ // ); -type SignatureStatus = 'needsSignature' | 'needsCustomHooks' | 'resolved'; export function createSignatureFunctionForTransform() { if (__DEV__) { - // We'll fill in the signature in two steps. - // First, we'll know the signature itself. This happens outside the component. - // Then, we'll know the references to custom Hooks. This happens inside the component. - // After that, the returned function will be a fast path no-op. - let status: SignatureStatus = 'needsSignature'; let savedType; let hasCustomHooks; + let didCollectHooks = false; return function( type: T, key: string, forceReset?: boolean, getCustomHooks?: () => Array, - ): T { - switch (status) { - case 'needsSignature': - if (type !== undefined) { - // If we received an argument, this is the initial registration call. - savedType = type; - hasCustomHooks = typeof getCustomHooks === 'function'; - setSignature(type, key, forceReset, getCustomHooks); - // The next call we expect is from inside a function, to fill in the custom Hooks. - status = 'needsCustomHooks'; - } - break; - case 'needsCustomHooks': - if (hasCustomHooks) { - collectCustomHooksForSignature(savedType); - } - status = 'resolved'; - break; - case 'resolved': - // Do nothing. Fast path for all future renders. - break; + ): T | void { + if (typeof key === 'string') { + // We're in the initial phase that associates signatures + // with the functions. Note this may be called multiple times + // in HOC chains like _s(hoc1(_s(hoc2(_s(actualFunction))))). + if (!savedType) { + // We're in the innermost call, so this is the actual type. + savedType = type; + hasCustomHooks = typeof getCustomHooks === 'function'; + } + // Set the signature for all types (even wrappers!) in case + // they have no signatures of their own. This is to prevent + // problems like https://github.com/facebook/react/issues/20417. + if ( + type != null && + (typeof type === 'function' || typeof type === 'object') + ) { + setSignature(type, key, forceReset, getCustomHooks); + } + return type; + } else { + // We're in the _s() call without arguments, which means + // this is the time to collect custom Hook signatures. + // Only do this once. This path is hot and runs *inside* every render! + if (!didCollectHooks && hasCustomHooks) { + didCollectHooks = true; + collectCustomHooksForSignature(savedType); + } } - return type; }; } else { throw new Error( diff --git a/packages/react-refresh/src/__tests__/ReactFreshBabelPlugin-test.js b/packages/react-refresh/src/__tests__/ReactFreshBabelPlugin-test.js index 320cfe17041a6..f56f16069dd77 100644 --- a/packages/react-refresh/src/__tests__/ReactFreshBabelPlugin-test.js +++ b/packages/react-refresh/src/__tests__/ReactFreshBabelPlugin-test.js @@ -524,4 +524,16 @@ describe('ReactFreshBabelPlugin', () => { '". If you want to override this check, pass {skipEnvCheck: true} as plugin options.', ); }); + + it('does not get tripped by IIFEs', () => { + expect( + transform(` + while (item) { + (item => { + useFoo(); + })(item); + } + `), + ).toMatchSnapshot(); + }); }); diff --git a/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js b/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js index f3538670e3717..231dd6b04219e 100644 --- a/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js +++ b/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js @@ -469,6 +469,227 @@ describe('ReactFreshIntegration', () => { } }); + it('resets state when renaming a state variable inside a HOC with direct call', () => { + if (__DEV__) { + render(` + const {useState} = React; + const S = 1; + + function hocWithDirectCall(Wrapped) { + return function Generated() { + return Wrapped(); + }; + } + + export default hocWithDirectCall(() => { + const [foo, setFoo] = useState(S); + return

A{foo}

; + }); + `); + const el = container.firstChild; + expect(el.textContent).toBe('A1'); + + patch(` + const {useState} = React; + const S = 2; + + function hocWithDirectCall(Wrapped) { + return function Generated() { + return Wrapped(); + }; + } + + export default hocWithDirectCall(() => { + const [foo, setFoo] = useState(S); + return

B{foo}

; + }); + `); + // Same state variable name, so state is preserved. + expect(container.firstChild).toBe(el); + expect(el.textContent).toBe('B1'); + + patch(` + const {useState} = React; + const S = 3; + + function hocWithDirectCall(Wrapped) { + return function Generated() { + return Wrapped(); + }; + } + + export default hocWithDirectCall(() => { + const [bar, setBar] = useState(S); + return

C{bar}

; + }); + `); + // Different state variable name, so state is reset. + expect(container.firstChild).not.toBe(el); + const newEl = container.firstChild; + expect(newEl.textContent).toBe('C3'); + } + }); + + it('does not crash when changing Hook order inside a HOC with direct call', () => { + if (__DEV__) { + render(` + const {useEffect} = React; + + function hocWithDirectCall(Wrapped) { + return function Generated() { + return Wrapped(); + }; + } + + export default hocWithDirectCall(() => { + useEffect(() => {}, []); + return

A

; + }); + `); + const el = container.firstChild; + expect(el.textContent).toBe('A'); + + patch(` + const {useEffect} = React; + + function hocWithDirectCall(Wrapped) { + return function Generated() { + return Wrapped(); + }; + } + + export default hocWithDirectCall(() => { + useEffect(() => {}, []); + useEffect(() => {}, []); + return

B

; + }); + `); + // Hook order changed, so we remount. + expect(container.firstChild).not.toBe(el); + const newEl = container.firstChild; + expect(newEl.textContent).toBe('B'); + } + }); + + it('does not crash when changing Hook order inside a memo-ed HOC with direct call', () => { + if (__DEV__) { + render(` + const {useEffect, memo} = React; + + function hocWithDirectCall(Wrapped) { + return memo(function Generated() { + return Wrapped(); + }); + } + + export default hocWithDirectCall(() => { + useEffect(() => {}, []); + return

A

; + }); + `); + const el = container.firstChild; + expect(el.textContent).toBe('A'); + + patch(` + const {useEffect, memo} = React; + + function hocWithDirectCall(Wrapped) { + return memo(function Generated() { + return Wrapped(); + }); + } + + export default hocWithDirectCall(() => { + useEffect(() => {}, []); + useEffect(() => {}, []); + return

B

; + }); + `); + // Hook order changed, so we remount. + expect(container.firstChild).not.toBe(el); + const newEl = container.firstChild; + expect(newEl.textContent).toBe('B'); + } + }); + + it('does not crash when changing Hook order inside a memo+forwardRef-ed HOC with direct call', () => { + if (__DEV__) { + render(` + const {useEffect, memo, forwardRef} = React; + + function hocWithDirectCall(Wrapped) { + return memo(forwardRef(function Generated() { + return Wrapped(); + })); + } + + export default hocWithDirectCall(() => { + useEffect(() => {}, []); + return

A

; + }); + `); + const el = container.firstChild; + expect(el.textContent).toBe('A'); + + patch(` + const {useEffect, memo, forwardRef} = React; + + function hocWithDirectCall(Wrapped) { + return memo(forwardRef(function Generated() { + return Wrapped(); + })); + } + + export default hocWithDirectCall(() => { + useEffect(() => {}, []); + useEffect(() => {}, []); + return

B

; + }); + `); + // Hook order changed, so we remount. + expect(container.firstChild).not.toBe(el); + const newEl = container.firstChild; + expect(newEl.textContent).toBe('B'); + } + }); + + it('does not crash when changing Hook order inside a HOC returning an object', () => { + if (__DEV__) { + render(` + const {useEffect} = React; + + function hocWithDirectCall(Wrapped) { + return {Wrapped: Wrapped}; + } + + export default hocWithDirectCall(() => { + useEffect(() => {}, []); + return

A

; + }).Wrapped; + `); + const el = container.firstChild; + expect(el.textContent).toBe('A'); + + patch(` + const {useEffect} = React; + + function hocWithDirectCall(Wrapped) { + return {Wrapped: Wrapped}; + } + + export default hocWithDirectCall(() => { + useEffect(() => {}, []); + useEffect(() => {}, []); + return

B

; + }).Wrapped; + `); + // Hook order changed, so we remount. + expect(container.firstChild).not.toBe(el); + const newEl = container.firstChild; + expect(newEl.textContent).toBe('B'); + } + }); + it('resets effects while preserving state', () => { if (__DEV__) { render(` diff --git a/packages/react-refresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap b/packages/react-refresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap index 729996f49ea3f..7c1276f817880 100644 --- a/packages/react-refresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap +++ b/packages/react-refresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap @@ -37,11 +37,13 @@ const Bar = () => { _s4(Bar, "useContext{}"); _c2 = Bar; -const Baz = memo(_c3 = _s5(() => { + +const Baz = _s5(memo(_c3 = _s5(() => { _s5(); return useContext(X); -}, "useContext{}")); +}, "useContext{}")), "useContext{}"); + _c4 = Baz; const Qux = () => { @@ -84,6 +86,18 @@ var _c; $RefreshReg$(_c, "App"); `; +exports[`ReactFreshBabelPlugin does not get tripped by IIFEs 1`] = ` +while (item) { + var _s = $RefreshSig$(); + + _s(item => { + _s(); + + useFoo(); + }, "useFoo{}", true)(item); +} +`; + exports[`ReactFreshBabelPlugin generates signatures for function declarations calling hooks 1`] = ` var _s = $RefreshSig$(); @@ -108,21 +122,21 @@ exports[`ReactFreshBabelPlugin generates signatures for function expressions cal var _s = $RefreshSig$(), _s2 = $RefreshSig$(); -export const A = React.memo(_c2 = React.forwardRef(_c = _s((props, ref) => { +export const A = _s(React.memo(_c2 = _s(React.forwardRef(_c = _s((props, ref) => { _s(); const [foo, setFoo] = useState(0); React.useEffect(() => {}); return

{foo}

; -}, "useState{[foo, setFoo](0)}\\nuseEffect{}"))); +}, "useState{[foo, setFoo](0)}\\nuseEffect{}")), "useState{[foo, setFoo](0)}\\nuseEffect{}")), "useState{[foo, setFoo](0)}\\nuseEffect{}"); _c3 = A; -export const B = React.memo(_c5 = React.forwardRef(_c4 = _s2(function (props, ref) { +export const B = _s2(React.memo(_c5 = _s2(React.forwardRef(_c4 = _s2(function (props, ref) { _s2(); const [foo, setFoo] = useState(0); React.useEffect(() => {}); return

{foo}

; -}, "useState{[foo, setFoo](0)}\\nuseEffect{}"))); +}, "useState{[foo, setFoo](0)}\\nuseEffect{}")), "useState{[foo, setFoo](0)}\\nuseEffect{}")), "useState{[foo, setFoo](0)}\\nuseEffect{}"); _c6 = B; function hoc() { From c9aab1c9d00ce407b1c61c385b356d49cd147f60 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 30 Mar 2021 16:19:57 +0100 Subject: [PATCH 07/18] react-refresh@0.10.0 --- packages/react-refresh/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-refresh/package.json b/packages/react-refresh/package.json index 84b3d704ee9d4..590bb19dea996 100644 --- a/packages/react-refresh/package.json +++ b/packages/react-refresh/package.json @@ -4,7 +4,7 @@ "keywords": [ "react" ], - "version": "0.9.0", + "version": "0.10.0", "homepage": "https://reactjs.org/", "bugs": "https://github.com/facebook/react/issues", "license": "MIT", From b48b38af68c27fd401fe4b923a8fa0b229693cd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Wed, 31 Mar 2021 11:22:49 -0400 Subject: [PATCH 08/18] Support nesting of startTransition and flushSync (alt) (#21149) * Support nesting of startTransition and flushSync * Unset transition before entering any special execution contexts Co-authored-by: Andrew Clark --- .../src/events/ReactDOMEventListener.js | 6 +++ .../src/ReactFiberWorkLoop.new.js | 22 ++++++++++ .../src/ReactFiberWorkLoop.old.js | 22 ++++++++++ .../src/__tests__/ReactFlushSync-test.js | 43 +++++++++++++++++++ 4 files changed, 93 insertions(+) diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js index 3f00dcf305b37..f09d6ee864481 100644 --- a/packages/react-dom/src/events/ReactDOMEventListener.js +++ b/packages/react-dom/src/events/ReactDOMEventListener.js @@ -57,6 +57,9 @@ import { getCurrentUpdatePriority, setCurrentUpdatePriority, } from 'react-reconciler/src/ReactEventPriorities'; +import ReactSharedInternals from 'shared/ReactSharedInternals'; + +const {ReactCurrentBatchConfig} = ReactSharedInternals; // TODO: can we stop exporting these? export let _enabled = true; @@ -141,11 +144,14 @@ function dispatchContinuousEvent( nativeEvent, ) { const previousPriority = getCurrentUpdatePriority(); + const prevTransition = ReactCurrentBatchConfig.transition; + ReactCurrentBatchConfig.transition = 0; try { setCurrentUpdatePriority(ContinuousEventPriority); dispatchEvent(domEventName, eventSystemFlags, container, nativeEvent); } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; } } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 1cff8db1fe654..37fa324f5a1a6 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -240,6 +240,7 @@ const ceil = Math.ceil; const { ReactCurrentDispatcher, ReactCurrentOwner, + ReactCurrentBatchConfig, IsSomeRendererActing, } = ReactSharedInternals; @@ -1072,11 +1073,14 @@ export function flushDiscreteUpdates() { export function deferredUpdates(fn: () => A): A { const previousPriority = getCurrentUpdatePriority(); + const prevTransition = ReactCurrentBatchConfig.transition; try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DefaultEventPriority); return fn(); } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; } } @@ -1118,11 +1122,14 @@ export function discreteUpdates( d: D, ): R { const previousPriority = getCurrentUpdatePriority(); + const prevTransition = ReactCurrentBatchConfig.transition; try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DiscreteEventPriority); return fn(a, b, c, d); } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; if (executionContext === NoContext) { // Flush the immediate callbacks that were scheduled during this batch resetRenderTimer(); @@ -1151,8 +1158,10 @@ export function flushSync(fn: A => R, a: A): R { const prevExecutionContext = executionContext; executionContext |= BatchedContext; + const prevTransition = ReactCurrentBatchConfig.transition; const previousPriority = getCurrentUpdatePriority(); try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DiscreteEventPriority); if (fn) { return fn(a); @@ -1161,6 +1170,7 @@ export function flushSync(fn: A => R, a: A): R { } } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; executionContext = prevExecutionContext; // Flush the immediate callbacks that were scheduled during this batch. // Note that this will happen even if batchedUpdates is higher up @@ -1182,12 +1192,15 @@ export function flushSync(fn: A => R, a: A): R { export function flushControlled(fn: () => mixed): void { const prevExecutionContext = executionContext; executionContext |= BatchedContext; + const prevTransition = ReactCurrentBatchConfig.transition; const previousPriority = getCurrentUpdatePriority(); try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DiscreteEventPriority); fn(); } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; executionContext = prevExecutionContext; if (executionContext === NoContext) { @@ -1681,10 +1694,13 @@ function commitRoot(root) { // TODO: This no longer makes any sense. We already wrap the mutation and // layout phases. Should be able to remove. const previousUpdateLanePriority = getCurrentUpdatePriority(); + const prevTransition = ReactCurrentBatchConfig.transition; try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DiscreteEventPriority); commitRootImpl(root, previousUpdateLanePriority); } finally { + ReactCurrentBatchConfig.transition = prevTransition; setCurrentUpdatePriority(previousUpdateLanePriority); } @@ -1797,6 +1813,8 @@ function commitRootImpl(root, renderPriorityLevel) { NoFlags; if (subtreeHasEffects || rootHasEffect) { + const prevTransition = ReactCurrentBatchConfig.transition; + ReactCurrentBatchConfig.transition = 0; const previousPriority = getCurrentUpdatePriority(); setCurrentUpdatePriority(DiscreteEventPriority); @@ -1882,6 +1900,7 @@ function commitRootImpl(root, renderPriorityLevel) { // Reset the priority to the previous non-sync value. setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; } else { // No effects. root.current = finishedWork; @@ -2019,12 +2038,15 @@ export function flushPassiveEffects(): boolean { DefaultEventPriority, lanesToEventPriority(pendingPassiveEffectsLanes), ); + const prevTransition = ReactCurrentBatchConfig.transition; const previousPriority = getCurrentUpdatePriority(); try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(priority); return flushPassiveEffectsImpl(); } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; } } return false; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 98f4d119e4088..f5102b96fe8bd 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -240,6 +240,7 @@ const ceil = Math.ceil; const { ReactCurrentDispatcher, ReactCurrentOwner, + ReactCurrentBatchConfig, IsSomeRendererActing, } = ReactSharedInternals; @@ -1072,11 +1073,14 @@ export function flushDiscreteUpdates() { export function deferredUpdates(fn: () => A): A { const previousPriority = getCurrentUpdatePriority(); + const prevTransition = ReactCurrentBatchConfig.transition; try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DefaultEventPriority); return fn(); } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; } } @@ -1118,11 +1122,14 @@ export function discreteUpdates( d: D, ): R { const previousPriority = getCurrentUpdatePriority(); + const prevTransition = ReactCurrentBatchConfig.transition; try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DiscreteEventPriority); return fn(a, b, c, d); } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; if (executionContext === NoContext) { // Flush the immediate callbacks that were scheduled during this batch resetRenderTimer(); @@ -1151,8 +1158,10 @@ export function flushSync(fn: A => R, a: A): R { const prevExecutionContext = executionContext; executionContext |= BatchedContext; + const prevTransition = ReactCurrentBatchConfig.transition; const previousPriority = getCurrentUpdatePriority(); try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DiscreteEventPriority); if (fn) { return fn(a); @@ -1161,6 +1170,7 @@ export function flushSync(fn: A => R, a: A): R { } } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; executionContext = prevExecutionContext; // Flush the immediate callbacks that were scheduled during this batch. // Note that this will happen even if batchedUpdates is higher up @@ -1182,12 +1192,15 @@ export function flushSync(fn: A => R, a: A): R { export function flushControlled(fn: () => mixed): void { const prevExecutionContext = executionContext; executionContext |= BatchedContext; + const prevTransition = ReactCurrentBatchConfig.transition; const previousPriority = getCurrentUpdatePriority(); try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DiscreteEventPriority); fn(); } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; executionContext = prevExecutionContext; if (executionContext === NoContext) { @@ -1681,10 +1694,13 @@ function commitRoot(root) { // TODO: This no longer makes any sense. We already wrap the mutation and // layout phases. Should be able to remove. const previousUpdateLanePriority = getCurrentUpdatePriority(); + const prevTransition = ReactCurrentBatchConfig.transition; try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DiscreteEventPriority); commitRootImpl(root, previousUpdateLanePriority); } finally { + ReactCurrentBatchConfig.transition = prevTransition; setCurrentUpdatePriority(previousUpdateLanePriority); } @@ -1797,6 +1813,8 @@ function commitRootImpl(root, renderPriorityLevel) { NoFlags; if (subtreeHasEffects || rootHasEffect) { + const prevTransition = ReactCurrentBatchConfig.transition; + ReactCurrentBatchConfig.transition = 0; const previousPriority = getCurrentUpdatePriority(); setCurrentUpdatePriority(DiscreteEventPriority); @@ -1882,6 +1900,7 @@ function commitRootImpl(root, renderPriorityLevel) { // Reset the priority to the previous non-sync value. setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; } else { // No effects. root.current = finishedWork; @@ -2019,12 +2038,15 @@ export function flushPassiveEffects(): boolean { DefaultEventPriority, lanesToEventPriority(pendingPassiveEffectsLanes), ); + const prevTransition = ReactCurrentBatchConfig.transition; const previousPriority = getCurrentUpdatePriority(); try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(priority); return flushPassiveEffectsImpl(); } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; } } return false; diff --git a/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js b/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js index 3c96f5ee65914..bc7499a3d43a7 100644 --- a/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js +++ b/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js @@ -3,6 +3,7 @@ let ReactNoop; let Scheduler; let useState; let useEffect; +let startTransition; describe('ReactFlushSync', () => { beforeEach(() => { @@ -13,6 +14,7 @@ describe('ReactFlushSync', () => { Scheduler = require('scheduler'); useState = React.useState; useEffect = React.useEffect; + startTransition = React.unstable_startTransition; }); function Text({text}) { @@ -54,4 +56,45 @@ describe('ReactFlushSync', () => { }); expect(root).toMatchRenderedOutput('1, 1'); }); + + // @gate experimental + test('nested with startTransition', async () => { + let setSyncState; + let setState; + function App() { + const [syncState, _setSyncState] = useState(0); + const [state, _setState] = useState(0); + setSyncState = _setSyncState; + setState = _setState; + return ; + } + + const root = ReactNoop.createRoot(); + await ReactNoop.act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['0, 0']); + expect(root).toMatchRenderedOutput('0, 0'); + + await ReactNoop.act(async () => { + ReactNoop.flushSync(() => { + startTransition(() => { + // This should be async even though flushSync is on the stack, because + // startTransition is closer. + setState(1); + ReactNoop.flushSync(() => { + // This should be async even though startTransition is on the stack, + // because flushSync is closer. + setSyncState(1); + }); + }); + }); + // Only the sync update should have flushed + expect(Scheduler).toHaveYielded(['1, 0']); + expect(root).toMatchRenderedOutput('1, 0'); + }); + // Now the async update has flushed, too. + expect(Scheduler).toHaveYielded(['1, 1']); + expect(root).toMatchRenderedOutput('1, 1'); + }); }); From f8ef4ff571db3de73b0bfab566c1ce9d69c6582f Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 31 Mar 2021 12:39:19 -0500 Subject: [PATCH 09/18] Flush discrete passive effects before paint (#21150) If a discrete render results in passive effects, we should flush them synchronously at the end of the current task so that the result is immediately observable. For example, if a passive effect adds an event listener, the listener will be added before the next input. We don't need to do this for effects that don't have discrete/sync priority, because we assume they are not order-dependent and do not need to be observed by external systems. For legacy mode, we will maintain the existing behavior, since it hasn't been reported as an issue, and we'd have to do additional work to distinguish "legacy default sync" from "discrete sync" to prevent all passive effects from being treated this way. --- .../src/ReactFiberWorkLoop.new.js | 15 ++++ .../src/ReactFiberWorkLoop.old.js | 15 ++++ .../src/__tests__/ReactFlushSync-test.js | 69 +++++++++++++++++++ .../ReactHooksWithNoopRenderer-test.js | 7 +- .../__tests__/ReactProfiler-test.internal.js | 11 ++- 5 files changed, 108 insertions(+), 9 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 37fa324f5a1a6..455067e47e263 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -2015,6 +2015,21 @@ function commitRootImpl(root, renderPriorityLevel) { return null; } + // If the passive effects are the result of a discrete render, flush them + // synchronously at the end of the current task so that the result is + // immediately observable. Otherwise, we assume that they are not + // order-dependent and do not need to be observed by external systems, so we + // can wait until after paint. + // TODO: We can optimize this by not scheduling the callback earlier. Since we + // currently schedule the callback in multiple places, will wait until those + // are consolidated. + if ( + includesSomeLane(pendingPassiveEffectsLanes, SyncLane) && + root.tag !== LegacyRoot + ) { + flushPassiveEffects(); + } + // If layout work was scheduled, flush it now. flushSyncCallbackQueue(); diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index f5102b96fe8bd..9d056a46402c3 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -2015,6 +2015,21 @@ function commitRootImpl(root, renderPriorityLevel) { return null; } + // If the passive effects are the result of a discrete render, flush them + // synchronously at the end of the current task so that the result is + // immediately observable. Otherwise, we assume that they are not + // order-dependent and do not need to be observed by external systems, so we + // can wait until after paint. + // TODO: We can optimize this by not scheduling the callback earlier. Since we + // currently schedule the callback in multiple places, will wait until those + // are consolidated. + if ( + includesSomeLane(pendingPassiveEffectsLanes, SyncLane) && + root.tag !== LegacyRoot + ) { + flushPassiveEffects(); + } + // If layout work was scheduled, flush it now. flushSyncCallbackQueue(); diff --git a/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js b/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js index bc7499a3d43a7..6326aaa2aeab6 100644 --- a/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js +++ b/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js @@ -97,4 +97,73 @@ describe('ReactFlushSync', () => { expect(Scheduler).toHaveYielded(['1, 1']); expect(root).toMatchRenderedOutput('1, 1'); }); + + test('flushes passive effects synchronously when they are the result of a sync render', async () => { + function App() { + useEffect(() => { + Scheduler.unstable_yieldValue('Effect'); + }, []); + return ; + } + + const root = ReactNoop.createRoot(); + await ReactNoop.act(async () => { + ReactNoop.flushSync(() => { + root.render(); + }); + expect(Scheduler).toHaveYielded([ + 'Child', + // Because the pending effect was the result of a sync update, calling + // flushSync should flush it. + 'Effect', + ]); + expect(root).toMatchRenderedOutput('Child'); + }); + }); + + test('do not flush passive effects synchronously in legacy mode', async () => { + function App() { + useEffect(() => { + Scheduler.unstable_yieldValue('Effect'); + }, []); + return ; + } + + const root = ReactNoop.createLegacyRoot(); + await ReactNoop.act(async () => { + ReactNoop.flushSync(() => { + root.render(); + }); + expect(Scheduler).toHaveYielded([ + 'Child', + // Because we're in legacy mode, we shouldn't have flushed the passive + // effects yet. + ]); + expect(root).toMatchRenderedOutput('Child'); + }); + // Effect flushes after paint. + expect(Scheduler).toHaveYielded(['Effect']); + }); + + test("do not flush passive effects synchronously when they aren't the result of a sync render", async () => { + function App() { + useEffect(() => { + Scheduler.unstable_yieldValue('Effect'); + }, []); + return ; + } + + const root = ReactNoop.createRoot(); + await ReactNoop.act(async () => { + root.render(); + expect(Scheduler).toFlushUntilNextPaint([ + 'Child', + // Because the passive effect was not the result of a sync update, it + // should not flush before paint. + ]); + expect(root).toMatchRenderedOutput('Child'); + }); + // Effect flushes after paint. + expect(Scheduler).toHaveYielded(['Effect']); + }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index cf4e4a3f3b619..973a0cbea81d7 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -33,6 +33,7 @@ let useDeferredValue; let forwardRef; let memo; let act; +let ContinuousEventPriority; describe('ReactHooksWithNoopRenderer', () => { beforeEach(() => { @@ -57,6 +58,8 @@ describe('ReactHooksWithNoopRenderer', () => { useDeferredValue = React.unstable_useDeferredValue; Suspense = React.Suspense; act = ReactNoop.act; + ContinuousEventPriority = require('react-reconciler/constants') + .ContinuousEventPriority; textCache = new Map(); @@ -1351,10 +1354,10 @@ describe('ReactHooksWithNoopRenderer', () => { expect(Scheduler).toFlushAndYieldThrough(['Child one render']); // Schedule unmount for the parent that unmounts children with pending update. - ReactNoop.flushSync(() => { + ReactNoop.unstable_runWithPriority(ContinuousEventPriority, () => { setParentState(false); }); - expect(Scheduler).toHaveYielded([ + expect(Scheduler).toFlushUntilNextPaint([ 'Parent false render', 'Parent false commit', ]); diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js index ff2897141b57f..ab7fbb73f025a 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -3764,13 +3764,10 @@ describe('Profiler', () => { ); }); - // Profiler tag causes passive effects to be scheduled, - // so the interactions are still not completed. - expect(Scheduler).toHaveYielded(['SecondComponent']); - expect(onInteractionTraced).toHaveBeenCalledTimes(2); - expect(onInteractionScheduledWorkCompleted).not.toHaveBeenCalled(); - expect(Scheduler).toFlushAndYieldThrough(['onPostCommit']); - + expect(Scheduler).toHaveYielded([ + 'SecondComponent', + 'onPostCommit', + ]); expect(onInteractionTraced).toHaveBeenCalledTimes(2); expect(onInteractionScheduledWorkCompleted).toHaveBeenCalledTimes( 1, From 0e96bdd4eefa0771fe1e7747bb79fd3c0e612b9c Mon Sep 17 00:00:00 2001 From: Lea Rosema Date: Thu, 1 Apr 2021 00:13:13 +0200 Subject: [PATCH 10/18] Remove my deadname from AUTHORS (#21152) --- AUTHORS | 1 - 1 file changed, 1 deletion(-) diff --git a/AUTHORS b/AUTHORS index a243f681bede4..146796383fbea 100644 --- a/AUTHORS +++ b/AUTHORS @@ -608,7 +608,6 @@ Ludovico Fischer Luigy Leon Luke Belliveau Luke Horvat -Lutz Rosema MICHAEL JACKSON MIKAMI Yoshiyuki Maciej Kasprzyk From b9e4c10e99940ceaff77e1f6f4cb0c747e91acff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Wed, 31 Mar 2021 20:39:38 -0400 Subject: [PATCH 11/18] [Fizz] Implement all the DOM attributes and special cases (#21153) * Implement DOM format config structure * Styles * Input warnings * Textarea special cases * Select special cases * Option special cases We read the currently selected value from the FormatContext. * Warning for non-lower case HTML We don't change to lower case at runtime anymore but keep the warning. * Pre tags innerHTML needs to be prefixed This is because if you do the equivalent on the client using innerHTML, this is the effect you'd get. * Extract errors --- .../src/__tests__/ReactDOMFizzServer-test.js | 2 +- .../src/server/ReactDOMServerFormatConfig.js | 1158 ++++++++++++++++- .../server/ReactNativeServerFormatConfig.js | 6 +- .../src/ReactNoopServer.js | 5 +- packages/react-server/src/ReactFizzServer.js | 10 +- scripts/error-codes/codes.json | 5 +- 6 files changed, 1142 insertions(+), 44 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 31b1f437c9871..0b69bf72231a3 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -428,7 +428,7 @@ describe('ReactDOMFizzServer', () => { } function AsyncCol({className}) { - return {[]}; + return ; } function AsyncPath({id}) { diff --git a/packages/react-dom/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom/src/server/ReactDOMServerFormatConfig.js index d42e92aa1e2b4..ae7f80f045fe9 100644 --- a/packages/react-dom/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom/src/server/ReactDOMServerFormatConfig.js @@ -7,6 +7,12 @@ * @flow */ +import type {ReactNodeList} from 'shared/ReactTypes'; + +import {Children} from 'react'; + +import {enableFilterEmptyStringAttributesDOM} from 'shared/ReactFeatureFlags'; + import type { Destination, Chunk, @@ -19,8 +25,29 @@ import { stringToPrecomputedChunk, } from 'react-server/src/ReactServerStreamConfig'; +import { + getPropertyInfo, + isAttributeNameSafe, + BOOLEAN, + OVERLOADED_BOOLEAN, + NUMERIC, + POSITIVE_NUMERIC, +} from '../shared/DOMProperty'; +import {isUnitlessNumber} from '../shared/CSSProperty'; + +import {checkControlledValueProps} from '../shared/ReactControlledValuePropTypes'; +import {validateProperties as validateARIAProperties} from '../shared/ReactDOMInvalidARIAHook'; +import {validateProperties as validateInputProperties} from '../shared/ReactDOMNullInputValuePropHook'; +import {validateProperties as validateUnknownProperties} from '../shared/ReactDOMUnknownPropertyHook'; +import warnValidStyle from '../shared/warnValidStyle'; + import escapeTextForBrowser from './escapeTextForBrowser'; +import hyphenateStyleName from '../shared/hyphenateStyleName'; import invariant from 'shared/invariant'; +import sanitizeURL from '../shared/sanitizeURL'; + +const hasOwnProperty = Object.prototype.hasOwnProperty; +const isArray = Array.isArray; // Per response, global state that is not contextual to the rendering subtree. export type ResponseState = { @@ -68,7 +95,7 @@ type InsertionMode = 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7; // Lets us keep track of contextual state and pick it back up after suspending. export type FormatContext = { insertionMode: InsertionMode, // root/svg/html/mathml/table - selectedValue: null | string, // the selected value(s) inside a + selectedValue: null | string | Array, // the selected value(s) inside a }; function createFormatContext( @@ -142,10 +169,6 @@ export function createSuspenseBoundaryID( return {formattedID: null}; } -function encodeHTMLIDAttribute(value: string): string { - return escapeTextForBrowser(value); -} - function encodeHTMLTextNode(text: string): string { return escapeTextForBrowser(text); } @@ -202,53 +225,1088 @@ export function pushTextInstance( target.push(stringToChunk(encodeHTMLTextNode(text)), textSeparator); } -const startTag1 = stringToPrecomputedChunk('<'); -const startTag2 = stringToPrecomputedChunk('>'); +const styleNameCache: Map = new Map(); +function processStyleName(styleName: string): PrecomputedChunk { + const chunk = styleNameCache.get(styleName); + if (chunk !== undefined) { + return chunk; + } + const result = stringToPrecomputedChunk( + escapeTextForBrowser(hyphenateStyleName(styleName)), + ); + styleNameCache.set(styleName, result); + return result; +} + +const styleAttributeStart = stringToPrecomputedChunk(' style="'); +const styleAssign = stringToPrecomputedChunk(':'); +const styleSeparator = stringToPrecomputedChunk(';'); + +function pushStyle( + target: Array, + responseState: ResponseState, + style: Object, +): void { + invariant( + typeof style === 'object', + 'The `style` prop expects a mapping from style properties to values, ' + + "not a string. For example, style={{marginRight: spacing + 'em'}} when " + + 'using JSX.', + ); + + let isFirst = true; + for (const styleName in style) { + if (!hasOwnProperty.call(style, styleName)) { + continue; + } + // If you provide unsafe user data here they can inject arbitrary CSS + // which may be problematic (I couldn't repro this): + // https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet + // http://www.thespanner.co.uk/2007/11/26/ultimate-xss-css-injection/ + // This is not an XSS hole but instead a potential CSS injection issue + // which has lead to a greater discussion about how we're going to + // trust URLs moving forward. See #2115901 + const styleValue = style[styleName]; + if ( + styleValue == null || + typeof styleValue === 'boolean' || + styleValue === '' + ) { + // TODO: We used to set empty string as a style with an empty value. Does that ever make sense? + continue; + } + + let nameChunk; + let valueChunk; + const isCustomProperty = styleName.indexOf('--') === 0; + if (isCustomProperty) { + nameChunk = stringToChunk(escapeTextForBrowser(styleName)); + valueChunk = stringToChunk( + escapeTextForBrowser(('' + styleValue).trim()), + ); + } else { + if (__DEV__) { + warnValidStyle(styleName, styleValue); + } + + nameChunk = processStyleName(styleName); + if (typeof styleValue === 'number') { + if ( + styleValue !== 0 && + !hasOwnProperty.call(isUnitlessNumber, styleName) + ) { + valueChunk = stringToChunk(styleValue + 'px'); // Presumes implicit 'px' suffix for unitless numbers + } else { + valueChunk = stringToChunk('' + styleValue); + } + } else { + valueChunk = stringToChunk(('' + styleValue).trim()); + } + } + if (isFirst) { + isFirst = false; + // If it's first, we don't need any separators prefixed. + target.push(styleAttributeStart, nameChunk, styleAssign, valueChunk); + } else { + target.push(styleSeparator, nameChunk, styleAssign, valueChunk); + } + } + if (!isFirst) { + target.push(attributeEnd); + } +} + +const attributeSeparator = stringToPrecomputedChunk(' '); +const attributeAssign = stringToPrecomputedChunk('="'); +const attributeEnd = stringToPrecomputedChunk('"'); +const attributeEmptyString = stringToPrecomputedChunk('=""'); + +function pushAttribute( + target: Array, + responseState: ResponseState, + name: string, + value: string | boolean | number | Function | Object, // not null or undefined +): void { + switch (name) { + case 'style': { + pushStyle(target, responseState, value); + return; + } + case 'defaultValue': + case 'defaultChecked': // These shouldn't be set as attributes on generic HTML elements. + case 'innerHTML': // Must use dangerouslySetInnerHTML instead. + case 'suppressContentEditableWarning': + case 'suppressHydrationWarning': + // Ignored. These are built-in to React on the client. + return; + } + if ( + // shouldIgnoreAttribute + // We have already filtered out null/undefined and reserved words. + name.length > 2 && + (name[0] === 'o' || name[0] === 'O') && + (name[1] === 'n' || name[1] === 'N') + ) { + return; + } + + const propertyInfo = getPropertyInfo(name); + if (propertyInfo !== null) { + // shouldRemoveAttribute + switch (typeof value) { + case 'function': + // $FlowIssue symbol is perfectly valid here + case 'symbol': // eslint-disable-line + return; + case 'boolean': { + if (!propertyInfo.acceptsBooleans) { + return; + } + } + } + if (enableFilterEmptyStringAttributesDOM) { + if (propertyInfo.removeEmptyString && value === '') { + if (__DEV__) { + if (name === 'src') { + console.error( + 'An empty string ("") was passed to the %s attribute. ' + + 'This may cause the browser to download the whole page again over the network. ' + + 'To fix this, either do not render the element at all ' + + 'or pass null to %s instead of an empty string.', + name, + name, + ); + } else { + console.error( + 'An empty string ("") was passed to the %s attribute. ' + + 'To fix this, either do not render the element at all ' + + 'or pass null to %s instead of an empty string.', + name, + name, + ); + } + } + return; + } + } + + const attributeName = propertyInfo.attributeName; + const attributeNameChunk = stringToChunk(attributeName); // TODO: If it's known we can cache the chunk. + + switch (propertyInfo.type) { + case BOOLEAN: + if (value) { + target.push( + attributeSeparator, + attributeNameChunk, + attributeEmptyString, + ); + } + return; + case OVERLOADED_BOOLEAN: + if (value === true) { + target.push( + attributeSeparator, + attributeNameChunk, + attributeEmptyString, + ); + } else if (value === false) { + // Ignored + } else { + target.push( + attributeSeparator, + attributeNameChunk, + attributeAssign, + escapeTextForBrowser(value), + attributeEnd, + ); + } + return; + case NUMERIC: + if (!isNaN(value)) { + target.push( + attributeSeparator, + attributeNameChunk, + attributeAssign, + escapeTextForBrowser(value), + attributeEnd, + ); + } + break; + case POSITIVE_NUMERIC: + if (!isNaN(value) && (value: any) >= 1) { + target.push( + attributeSeparator, + attributeNameChunk, + attributeAssign, + escapeTextForBrowser(value), + attributeEnd, + ); + } + break; + default: + if (propertyInfo.sanitizeURL) { + value = '' + (value: any); + sanitizeURL(value); + } + target.push( + attributeSeparator, + attributeNameChunk, + attributeAssign, + escapeTextForBrowser(value), + attributeEnd, + ); + } + } else if (isAttributeNameSafe(name)) { + // shouldRemoveAttribute + switch (typeof value) { + case 'function': + // $FlowIssue symbol is perfectly valid here + case 'symbol': // eslint-disable-line + return; + case 'boolean': { + const prefix = name.toLowerCase().slice(0, 5); + if (prefix !== 'data-' && prefix !== 'aria-') { + return; + } + } + } + target.push( + attributeSeparator, + stringToChunk(name), + attributeAssign, + escapeTextForBrowser(value), + attributeEnd, + ); + } +} + +const endOfStartTag = stringToPrecomputedChunk('>'); +const endOfStartTagSelfClosing = stringToPrecomputedChunk('/>'); const idAttr = stringToPrecomputedChunk(' id="'); const attrEnd = stringToPrecomputedChunk('"'); -export function pushStartInstance( +function pushID( + target: Array, + responseState: ResponseState, + assignID: SuspenseBoundaryID, + existingID: mixed, +): void { + if ( + existingID !== null && + existingID !== undefined && + (typeof existingID === 'string' || typeof existingID === 'object') + ) { + // We can reuse the existing ID for our purposes. + assignID.formattedID = stringToPrecomputedChunk( + escapeTextForBrowser(existingID), + ); + } else { + const encodedID = assignAnID(responseState, assignID); + target.push(idAttr, encodedID, attrEnd); + } +} + +function pushInnerHTML( + target: Array, + innerHTML, + children, +) { + if (innerHTML != null) { + invariant( + children == null, + 'Can only set one of `children` or `props.dangerouslySetInnerHTML`.', + ); + + invariant( + typeof innerHTML === 'object' && '__html' in innerHTML, + '`props.dangerouslySetInnerHTML` must be in the form `{__html: ...}`. ' + + 'Please visit https://reactjs.org/link/dangerously-set-inner-html ' + + 'for more information.', + ); + const html = innerHTML.__html; + target.push(stringToChunk(html)); + } +} + +// TODO: Move these to ResponseState so that we warn for every request. +// It would help debugging in stateful servers (e.g. service worker). +let didWarnDefaultInputValue = false; +let didWarnDefaultChecked = false; +let didWarnDefaultSelectValue = false; +let didWarnDefaultTextareaValue = false; +let didWarnInvalidOptionChildren = false; +let didWarnSelectedSetOnOption = false; + +function checkSelectProp(props, propName) { + if (__DEV__) { + const array = isArray(props[propName]); + if (props.multiple && !array) { + console.error( + 'The `%s` prop supplied to must be a scalar ' + + 'value if `multiple` is false.', + propName, + ); + } + } +} + +function pushStartSelect( target: Array, - type: string, props: Object, responseState: ResponseState, assignID: null | SuspenseBoundaryID, -): void { - // TODO: Figure out if it's self closing and everything else. - if (assignID !== null) { - let encodedID; - if (typeof props.id === 'string') { - // We can reuse the existing ID for our purposes. - encodedID = assignID.formattedID = stringToPrecomputedChunk( - encodeHTMLIDAttribute(props.id), +): ReactNodeList { + if (__DEV__) { + checkControlledValueProps('select', props); + + checkSelectProp(props, 'value'); + checkSelectProp(props, 'defaultValue'); + + if ( + props.value !== undefined && + props.defaultValue !== undefined && + !didWarnDefaultSelectValue + ) { + console.error( + 'Select elements must be either controlled or uncontrolled ' + + '(specify either the value prop, or the defaultValue prop, but not ' + + 'both). Decide between using a controlled or uncontrolled select ' + + 'element and remove one of these props. More info: ' + + 'https://reactjs.org/link/controlled-components', ); + didWarnDefaultSelectValue = true; + } + } + + target.push(startChunkForTag('select')); + + let children = null; + let innerHTML = null; + for (const propKey in props) { + if (hasOwnProperty.call(props, propKey)) { + const propValue = props[propKey]; + if (propValue == null) { + continue; + } + switch (propKey) { + case 'children': + children = propValue; + break; + case 'dangerouslySetInnerHTML': + // TODO: This doesn't really make sense for select since it can't use the controlled + // value in the innerHTML. + innerHTML = propValue; + break; + case 'defaultValue': + case 'value': + // These are set on the Context instead and applied to the nested options. + break; + default: + pushAttribute(target, responseState, propKey, propValue); + break; + } + } + } + if (assignID !== null) { + pushID(target, responseState, assignID, props.id); + } + + target.push(endOfStartTag); + pushInnerHTML(target, innerHTML, children); + return children; +} + +function flattenOptionChildren(children: mixed): string { + let content = ''; + // Flatten children and warn if they aren't strings or numbers; + // invalid types are ignored. + Children.forEach((children: any), function(child) { + if (child == null) { + return; + } + content += (child: any); + if (__DEV__) { + if ( + !didWarnInvalidOptionChildren && + typeof child !== 'string' && + typeof child !== 'number' + ) { + didWarnInvalidOptionChildren = true; + console.error( + 'Only strings and numbers are supported as