From 0e8255f732648ea3f6ecfd5ec6bc4b19a96bcec9 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Tue, 7 Feb 2023 13:44:15 -0800 Subject: [PATCH] cleanup --- .prettierrc.js | 2 +- .../src/client/ReactDOMFloatClient.js | 27 +-- .../src/client/ReactDOMHostConfig.js | 50 +++-- .../src/server/ReactDOMServerFormatConfig.js | 156 +++++++------ .../ReactDOMServerLegacyFormatConfig.js | 1 + ...actDOMFizzInstructionSetExternalRuntime.js | 2 +- .../ReactDOMFizzInstructionSetInlineSource.js | 2 +- .../src/__tests__/ReactDOMComponent-test.js | 1 + .../src/__tests__/ReactDOMFizzServer-test.js | 206 ++++++++---------- .../ReactDOMFizzServerBrowser-test.js | 12 +- .../__tests__/ReactDOMFizzServerNode-test.js | 13 +- .../ReactDOMFizzStaticBrowser-test.js | 12 +- .../__tests__/ReactDOMFizzStaticNode-test.js | 12 +- .../src/__tests__/ReactDOMFloat-test.js | 130 ++++++++++- .../src/__tests__/ReactDOMRoot-test.js | 23 +- .../ReactDOMSingletonComponents-test.js | 10 +- .../react-dom-server-rendering-stub-test.js | 2 +- .../server/ReactNativeServerFormatConfig.js | 12 +- .../src/ReactNoopServer.js | 2 - .../src/ReactFiberBeginWork.js | 67 +++--- .../src/ReactFiberCommitWork.js | 39 ++-- .../src/ReactFiberCompleteWork.js | 58 +++-- .../ReactFiberHostConfigWithNoResources.js | 7 + .../src/ReactFiberWorkLoop.js | 16 +- .../src/forks/ReactFiberHostConfig.custom.js | 6 + .../forks/ReactServerFormatConfig.custom.js | 2 + packages/shared/ReactFeatureFlags.js | 3 +- scripts/error-codes/codes.json | 2 +- 28 files changed, 520 insertions(+), 355 deletions(-) diff --git a/.prettierrc.js b/.prettierrc.js index 7109a6fe8c57b..4f7ef193130c9 100644 --- a/.prettierrc.js +++ b/.prettierrc.js @@ -8,7 +8,7 @@ module.exports = { jsxBracketSameLine: true, trailingComma: 'es5', printWidth: 80, - parser: 'babel', + parser: 'flow', arrowParens: 'avoid', overrides: [ { diff --git a/packages/react-dom-bindings/src/client/ReactDOMFloatClient.js b/packages/react-dom-bindings/src/client/ReactDOMFloatClient.js index 73b18a3aff76c..10c44364c5b60 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMFloatClient.js +++ b/packages/react-dom-bindings/src/client/ReactDOMFloatClient.js @@ -16,13 +16,6 @@ import ReactDOMSharedInternals from 'shared/ReactDOMSharedInternals.js'; const {Dispatcher} = ReactDOMSharedInternals; import {DOCUMENT_NODE} from '../shared/HTMLNodeType'; import { - warnOnMissingHrefAndRel, - validatePreloadResourceDifference, - validateURLKeyedUpdatedProps, - validateStyleResourceDifference, - validateScriptResourceDifference, - validateLinkPropsForStyleResource, - validateLinkPropsForPreloadResource, validatePreloadArguments, validatePreinitArguments, } from '../shared/ReactDOMResourceValidation'; @@ -178,9 +171,8 @@ function preload(href: string, options: PreloadOptions) { ownerDocument ) { const as = options.as; - const limitedEscapedHref = escapeSelectorAttributeValueInsideDoubleQuotes( - href, - ); + const limitedEscapedHref = + escapeSelectorAttributeValueInsideDoubleQuotes(href); const preloadKey = `link[rel="preload"][as="${as}"][href="${limitedEscapedHref}"]`; let key = preloadKey; switch (as) { @@ -256,9 +248,8 @@ function preinit(href: string, options: PreinitOptions) { // matching preload with this href const preloadDocument = getDocumentForPreloads(); if (preloadDocument) { - const limitedEscapedHref = escapeSelectorAttributeValueInsideDoubleQuotes( - href, - ); + const limitedEscapedHref = + escapeSelectorAttributeValueInsideDoubleQuotes(href); const preloadKey = `link[rel="preload"][as="${as}"][href="${limitedEscapedHref}"]`; let key = preloadKey; switch (as) { @@ -476,8 +467,6 @@ export function getResource( const styles = getResourcesFromRoot(resourceRoot).hoistableStyles; let resource = styles.get(key); if (!resource) { - // We asserted this above but Flow can't figure out that the type satisfies - const ownerDocument = getDocumentFromRoot(resourceRoot); resource = { type: 'style', instance: null, @@ -570,9 +559,8 @@ function styleTagPropsFromRawProps( } function getStyleKey(href: string) { - const limitedEscapedHref = escapeSelectorAttributeValueInsideDoubleQuotes( - href, - ); + const limitedEscapedHref = + escapeSelectorAttributeValueInsideDoubleQuotes(href); return `href="${limitedEscapedHref}"`; } @@ -609,9 +597,6 @@ function preloadStylesheet( // There is no matching stylesheet instance in the Document. // We will insert a preload now to kick off loading because // we expect this stylesheet to commit - const limitedEscapedHref = escapeSelectorAttributeValueInsideDoubleQuotes( - preloadProps.href, - ); if ( null === ownerDocument.querySelector(getPreloadStylesheetSelectorFromKey(key)) diff --git a/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js b/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js index dfcd38e561fba..281cc2a118ec0 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js @@ -463,10 +463,7 @@ export const scheduleMicrotask: any = ? queueMicrotask : typeof localPromise !== 'undefined' ? callback => - localPromise - .resolve(null) - .then(callback) - .catch(handleErrorInNextTick) + localPromise.resolve(null).then(callback).catch(handleErrorInNextTick) : scheduleTimeout; // TODO: Determine the best fallback here. function handleErrorInNextTick(error: any) { @@ -1591,8 +1588,8 @@ export function isHostHoistableType( if (__DEV__) { const hostContextDev: HostContextDev = (hostContext: any); // We can only render resources when we are not within the host container context - outsideHostContainerContext = !hostContextDev.ancestorInfo - .containerTagInScope; + outsideHostContainerContext = + !hostContextDev.ancestorInfo.containerTagInScope; namespace = hostContextDev.namespace; } else { const hostContextProd: HostContextProd = (hostContext: any); @@ -1623,22 +1620,40 @@ export function isHostHoistableType( ); } case 'link': { - const {onLoad, onError} = props; - if (onLoad || onError) { + const {onLoad, onError, rel, href} = props; + if ( + namespace === SVG_NAMESPACE || + typeof rel !== 'string' || + typeof href !== 'string' || + href === '' || + onLoad || + onError + ) { if (__DEV__) { if (outsideHostContainerContext) { - console.error( - 'Cannot render a with onLoad or onError listeners outside the main document.' + - ' Try removing onLoad={...} and onError={...} or moving it into the root tag or' + - ' somewhere in the .', - ); + if ( + typeof rel !== 'string' || + typeof href !== 'string' || + href === '' + ) { + console.error( + 'Cannot render a outside the main document without a `rel` and `href` prop.' + + ' Try adding a `rel` and/or `href` prop to this or moving the link into the tag', + ); + } else if (onError || onLoad) { + console.error( + 'Cannot render a with onLoad or onError listeners outside the main document.' + + ' Try removing onLoad={...} and onError={...} or moving it into the root tag or' + + ' somewhere in the .', + ); + } } } return false; } switch (props.rel) { case 'stylesheet': { - const {href, precedence, disabled} = props; + const {precedence, disabled} = props; if (__DEV__) { validateLinkPropsForStyleResource(props); if (typeof precedence !== 'string') { @@ -1650,12 +1665,7 @@ export function isHostHoistableType( } } } - return ( - namespace !== SVG_NAMESPACE && - typeof href === 'string' && - typeof precedence === 'string' && - disabled == null - ); + return typeof precedence === 'string' && disabled == null; } default: { return true; diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js index 1152782e83d9a..cc1029c92d811 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js @@ -892,7 +892,7 @@ 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) { + Children.forEach((children: any), function (child) { if (child == null) { return; } @@ -1355,8 +1355,8 @@ function pushLink( // This stylesheet refers to a Resource and we create a new one if necessary let resource = resources.stylesMap.get(key); if (__DEV__) { - if (resource) { - const devResource: ResourceDEV = (resource: any); + const devResource = getAsResourceDEV(resource); + if (devResource) { switch (devResource.__provenance) { case 'rendered': { const differentProps = compareResourcePropsForWarning( @@ -1365,9 +1365,8 @@ function pushLink( devResource.__originalProps, ); if (differentProps) { - const differenceDescription = describeDifferencesForStylesheets( - differentProps, - ); + const differenceDescription = + describeDifferencesForStylesheets(differentProps); if (differenceDescription) { console.error( 'React encountered a with a `precedence` prop that has props that conflict' + @@ -1388,9 +1387,10 @@ function pushLink( devResource.__propsEquivalent, ); if (differentProps) { - const differenceDescription = describeDifferencesForStylesheetOverPreinit( - differentProps, - ); + const differenceDescription = + describeDifferencesForStylesheetOverPreinit( + differentProps, + ); if (differenceDescription) { console.error( 'React encountered a with props that conflict' + @@ -1725,22 +1725,40 @@ function pushTitle( : null : children; - if ( - typeof child === 'function' || - typeof child === 'symbol' || - Array.isArray(child) - ) { + if (Array.isArray(children) && children.length > 1) { + console.error( + 'React expects the `children` prop of tags to be a string, number, or object with a novel `toString` method but found an Array with length %s instead.' + + ' Browsers treat all child Nodes of <title> tags as Text content and React expects to be able to convert `children` of <title> tags to a single string value' + + ' which is why Arrays of length greater than 1 are not supported. When using JSX it can be commong to combine text nodes and value nodes.' + + ' For example: <title>hello {nameOfUser}. While not immediately apparent, `children` in this case is an Array with length 2. If your `children` prop' + + ' is using this form try rewriting it using a template string: {`hello ${nameOfUser}`}.', + children.length, + ); + } else if (typeof child === 'function' || typeof child === 'symbol') { const childType = - typeof child === 'function' - ? 'a Function' - : typeof child === 'symbol' - ? 'a Sybmol' - : 'an Array'; + typeof child === 'function' ? 'a Function' : 'a Sybmol'; console.error( - 'React expect children of tags to be a string, number, or object with a `toString` method but found %s instead. ' + - 'In browsers title Elements can only have `Text` Nodes as children.', + 'React expect children of <title> tags to be a string, number, or object with a novel `toString` method but found %s instead.' + + ' Browsers treat all child Nodes of <title> tags as Text content and React expects to be able to convert children of <title>' + + ' tags to a single string value.', childType, ); + } else if (child && child.toString === {}.toString) { + if (child.$$typeof != null) { + console.error( + 'React expects the `children` prop of <title> tags to be a string, number, or object with a novel `toString` method but found an object that appears to be' + + ' a React element which never implements a suitable `toString` method. Browsers treat all child Nodes of <title> tags as Text content and React expects to' + + ' be able to convert children of <title> tags to a single string value which is why rendering React elements is not supported. If the `children` of <title> is' + + ' a React Component try moving the <title> tag into that component. If the `children` of <title> is some HTML markup change it to be Text only to be valid HTML.', + ); + } else { + console.error( + 'React expects the `children` prop of <title> tags to be a string, number, or object with a novel `toString` method but found an object that does not implement' + + ' a suitable `toString` method. Browsers treat all child Nodes of <title> tags as Text content and React expects to be able to convert children of <title> tags' + + ' to a single string value. Using the default `toString` method available on every object is almost certainly an error. Consider whether the `children` of this <title>' + + ' is an object in error and change it to a string or number value if so. Otherwise implement a `toString` method that React can use to produce a valid <title>.', + ); + } } } } @@ -2535,29 +2553,22 @@ const startPendingSuspenseBoundary1 = stringToPrecomputedChunk( '<!--$?--><template id="', ); const startPendingSuspenseBoundary2 = stringToPrecomputedChunk('"></template>'); -const startClientRenderedSuspenseBoundary = stringToPrecomputedChunk( - '<!--$!-->', -); +const startClientRenderedSuspenseBoundary = + stringToPrecomputedChunk('<!--$!-->'); const endSuspenseBoundary = stringToPrecomputedChunk('<!--/$-->'); -const clientRenderedSuspenseBoundaryError1 = stringToPrecomputedChunk( - '<template', -); -const clientRenderedSuspenseBoundaryErrorAttrInterstitial = stringToPrecomputedChunk( - '"', -); -const clientRenderedSuspenseBoundaryError1A = stringToPrecomputedChunk( - ' data-dgst="', -); -const clientRenderedSuspenseBoundaryError1B = stringToPrecomputedChunk( - ' data-msg="', -); -const clientRenderedSuspenseBoundaryError1C = stringToPrecomputedChunk( - ' data-stck="', -); -const clientRenderedSuspenseBoundaryError2 = stringToPrecomputedChunk( - '></template>', -); +const clientRenderedSuspenseBoundaryError1 = + stringToPrecomputedChunk('<template'); +const clientRenderedSuspenseBoundaryErrorAttrInterstitial = + stringToPrecomputedChunk('"'); +const clientRenderedSuspenseBoundaryError1A = + stringToPrecomputedChunk(' data-dgst="'); +const clientRenderedSuspenseBoundaryError1B = + stringToPrecomputedChunk(' data-msg="'); +const clientRenderedSuspenseBoundaryError1C = + stringToPrecomputedChunk(' data-stck="'); +const clientRenderedSuspenseBoundaryError2 = + stringToPrecomputedChunk('></template>'); export function pushStartCompletedSuspenseBoundary( target: Array<Chunk | PrecomputedChunk>, @@ -2861,9 +2872,8 @@ const completeBoundaryWithStylesScript1FullBoth = stringToPrecomputedChunk( const completeBoundaryWithStylesScript1FullPartial = stringToPrecomputedChunk( styleInsertionFunction + '$RR("', ); -const completeBoundaryWithStylesScript1Partial = stringToPrecomputedChunk( - '$RR("', -); +const completeBoundaryWithStylesScript1Partial = + stringToPrecomputedChunk('$RR("'); const completeBoundaryScript2 = stringToPrecomputedChunk('","'); const completeBoundaryScript3a = stringToPrecomputedChunk('",'); const completeBoundaryScript3b = stringToPrecomputedChunk('"'); @@ -3150,11 +3160,20 @@ const styleTagTemplateOpen = stringToPrecomputedChunk( ); const styleTagTemplateClose = stringToPrecomputedChunk('</template>'); +// Tracks whether we wrote any late style tags. We use this to determine +// whether we need to emit a closing template tag after flushing late style tags +let didWrite = false; + function flushStyleTagsLateForBoundary( this: Destination, resource: StyleResource, ) { if (resource.type === 'style' && (resource.state & Flushed) === NoState) { + if (didWrite === false) { + // we are going to write so we need to emit the open tag + didWrite = true; + writeChunk(this, styleTagTemplateOpen); + } // This <style> tag can be flushed now const chunks = resource.chunks; for (let i = 0; i < chunks.length; i++) { @@ -3168,9 +3187,13 @@ export function writeResourcesForBoundary( destination: Destination, boundaryResources: BoundaryResources, ): boolean { - writeChunk(destination, styleTagTemplateOpen); + didWrite = false; boundaryResources.forEach(flushStyleTagsLateForBoundary, destination); - return writeChunkAndReturn(destination, styleTagTemplateClose); + if (didWrite) { + return writeChunkAndReturn(destination, styleTagTemplateClose); + } else { + return true; + } } const precedencePlaceholderStart = stringToPrecomputedChunk( @@ -4278,9 +4301,8 @@ function preinitImpl( devResource.__originalProps, ); if (differentProps) { - const differenceDescription = describeDifferencesForPreinitOverStylesheet( - differentProps, - ); + const differenceDescription = + describeDifferencesForPreinitOverStylesheet(differentProps); if (differenceDescription) { console.error( 'ReactDOM.preinit(): For `href` "%s", the options provided conflict with props found on a <link rel="stylesheet" precedence="%s" href="%s" .../> that was already rendered.' + @@ -4305,9 +4327,8 @@ function preinitImpl( devResource.__propsEquivalent, ); if (differentProps) { - const differenceDescription = describeDifferencesForPreinits( - differentProps, - ); + const differenceDescription = + describeDifferencesForPreinits(differentProps); if (differenceDescription) { console.error( 'ReactDOM.preinit(): For `href` "%s", the options provided conflict with another call to `ReactDOM.preinit("%s", { as: "style", ... })`.' + @@ -4360,7 +4381,7 @@ function preinitImpl( props: null, }; resources.scriptsMap.set(key, resource); - let resourceProps = scriptPropsFromPreinitOptions(src, options); + const resourceProps = scriptPropsFromPreinitOptions(src, options); if (__DEV__) { markAsImperativeResourceDEV( resource, @@ -4518,13 +4539,14 @@ function markAsRenderedResourceDEV( if (__DEV__) { const devResource: RenderedResourceDEV = (resource: any); if (typeof devResource.__provenance === 'string') { - throw new Error( + console.error( 'Resource already marked for DEV type. This is a bug in React.', ); } devResource.__provenance = 'rendered'; devResource.__originalProps = originalProps; } else { + // eslint-disable-next-line react-internal/prod-error-codes throw new Error( 'markAsRenderedResourceDEV was included in a production build. This is a bug in React.', ); @@ -4541,7 +4563,7 @@ function markAsImperativeResourceDEV( if (__DEV__) { const devResource: ImperativeResourceDEV = (resource: any); if (typeof devResource.__provenance === 'string') { - throw new Error( + console.error( 'Resource already marked for DEV type. This is a bug in React.', ); } @@ -4550,6 +4572,7 @@ function markAsImperativeResourceDEV( devResource.__originalOptions = originalOptions; devResource.__propsEquivalent = propsEquivalent; } else { + // eslint-disable-next-line react-internal/prod-error-codes throw new Error( 'markAsImperativeResourceDEV was included in a production build. This is a bug in React.', ); @@ -4564,7 +4587,7 @@ function markAsImplicitResourceDEV( if (__DEV__) { const devResource: ImplicitResourceDEV = (resource: any); if (typeof devResource.__provenance === 'string') { - throw new Error( + console.error( 'Resource already marked for DEV type. This is a bug in React.', ); } @@ -4572,21 +4595,28 @@ function markAsImplicitResourceDEV( devResource.__underlyingProps = underlyingProps; devResource.__impliedProps = impliedProps; } else { + // eslint-disable-next-line react-internal/prod-error-codes throw new Error( 'markAsImplicitResourceDEV was included in a production build. This is a bug in React.', ); } } -function getAsResourceDEV(resource: Resource): ResourceDEV { +function getAsResourceDEV( + resource: null | void | Resource, +): null | ResourceDEV { if (__DEV__) { - if (typeof (resource: any).__provenance === 'string') { - return (resource: any); + if (resource) { + if (typeof (resource: any).__provenance === 'string') { + return (resource: any); + } + console.error( + 'Resource was not marked for DEV type. This is a bug in React.', + ); } - throw new Error( - 'Resource was not marked for DEV type. This is a bug in React.', - ); + return null; } else { + // eslint-disable-next-line react-internal/prod-error-codes throw new Error( 'getAsResourceDEV was included in a production build. This is a bug in React.', ); diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerLegacyFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerLegacyFormatConfig.js index 30cb1a376ed80..c965175e7e8f0 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerLegacyFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerLegacyFormatConfig.js @@ -125,6 +125,7 @@ export { writeClientRenderBoundaryInstruction, writeStartPendingSuspenseBoundary, writeEndPendingSuspenseBoundary, + writeResourcesForBoundary, writePlaceholder, writeCompletedRoot, createResources, diff --git a/packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetExternalRuntime.js b/packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetExternalRuntime.js index 5450fcbf25340..d6698c427fc27 100644 --- a/packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetExternalRuntime.js +++ b/packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetExternalRuntime.js @@ -30,7 +30,7 @@ export function completeBoundaryWithStyles( let nodes = thisDocument.querySelectorAll('template[data-precedence]'); for (let i = 0; (node = nodes[i++]); ) { let child = node.content.firstChild; - for (let j = 0; child; child = child.nextSibling) { + for (; child; child = child.nextSibling) { stylesToHoist.set(child.getAttribute('data-href'), child); } node.parentNode.removeChild(node); diff --git a/packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineSource.js b/packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineSource.js index e125eb8a49a9b..84652fdd8c464 100644 --- a/packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineSource.js +++ b/packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineSource.js @@ -33,7 +33,7 @@ export function completeBoundaryWithStyles( let nodes = thisDocument.querySelectorAll('template[data-precedence]'); for (let i = 0; (node = nodes[i++]); ) { let child = node.content.firstChild; - for (let j = 0; child; child = child.nextSibling) { + for (; child; child = child.nextSibling) { stylesToHoist.set(child.getAttribute('data-href'), child); } node.parentNode.removeChild(node); diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js index b474103f97ad3..946c5b2f3367b 100644 --- a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js @@ -494,6 +494,7 @@ describe('ReactDOMComponent', () => { 'To fix this, either do not render the element at all ' + 'or pass null to href instead of an empty string.', ); + console.log('container', container.outerHTML); const node = container.firstChild; expect(node.hasAttribute('href')).toBe(false); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index a21e1d593cabc..a9f5528f0df65 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -4611,6 +4611,7 @@ describe('ReactDOMFizzServer', () => { expect(div.outerHTML).toEqual( '<div id="app-div">hello<b>world, Foo</b>!</div>', ); + // there may be either: // - an external runtime script and deleted nodes with data attributes // - extra script nodes containing fizz instructions at the end of container @@ -5022,154 +5023,117 @@ describe('ReactDOMFizzServer', () => { }); it('should warn in dev when given an array of length 2 or more', async () => { - const originalConsoleError = console.error; - const mockError = jest.fn(); - console.error = (...args) => { - if (args.length > 1) { - if (typeof args[1] === 'object') { - mockError(args[0].split('\n')[0]); - return; - } - } - mockError(...args.map(normalizeCodeLocInfo)); - }; - - // a Single string child function App() { return <title>{['hello1', 'hello2']}; } - try { - prepareJSDOMForTitle(); + prepareJSDOMForTitle(); + await expect(async () => { await act(async () => { const {pipe} = renderToPipeableStream(); pipe(writable); }); - if (__DEV__) { - expect(mockError).toHaveBeenCalledWith( - 'Warning: A title element received an array with more than 1 element as children. ' + - 'In browsers title Elements can only have Text Nodes as children. If ' + - 'the children being rendered output more than a single text node in aggregate the browser ' + - 'will display markup and comments as text in the title and hydration will likely fail and ' + - 'fall back to client rendering%s', - '\n' + ' in title (at **)\n' + ' in App (at **)', - ); - } else { - expect(mockError).not.toHaveBeenCalled(); - } + }).toErrorDev([ + 'React expects the `children` prop of tags to be a string, number, or object with a novel `toString` method but found an Array with length 2 instead. Browsers treat all child Nodes of <title> tags as Text content and React expects to be able to convert `children` of <title> tags to a single string value which is why Arrays of length greater than 1 are not supported. When using JSX it can be commong to combine text nodes and value nodes. For example: <title>hello {nameOfUser}. While not immediately apparent, `children` in this case is an Array with length 2. If your `children` prop is using this form try rewriting it using a template string: {`hello ${nameOfUser}`}.', + ]); - if (gate(flags => flags.enableFloat)) { - // This title was invalid so it is not emitted - expect(getVisibleChildren(container)).toEqual(undefined); - } else { - expect(getVisibleChildren(container)).toEqual( - {'hello1<!-- -->hello2'}, - ); - } + if (gate(flags => flags.enableFloat)) { + expect(getVisibleChildren(container)).toEqual(); + } else { + expect(getVisibleChildren(container)).toEqual( + <title>{'hello1<!-- -->hello2'}, + ); + } - const errors = []; - ReactDOMClient.hydrateRoot(container, , { - onRecoverableError(error) { - errors.push(error.message); - }, - }); - expect(Scheduler).toFlushAndYield([]); - if (gate(flags => flags.enableFloat)) { - expect(errors).toEqual([]); - // with float, the title doesn't render on the client or on the server - expect(getVisibleChildren(container)).toEqual(undefined); - } else { - expect(errors).toEqual( - [ - gate(flags => flags.enableClientRenderFallbackOnTextMismatch) - ? 'Text content does not match server-rendered HTML.' - : null, - 'Hydration failed because the initial UI does not match what was rendered on the server.', - 'There was an error while hydrating. Because the error happened outside of a Suspense boundary, the entire root will switch to client rendering.', - ].filter(Boolean), - ); - expect(getVisibleChildren(container)).toEqual( - {['hello1', 'hello2']}, - ); - } - } finally { - console.error = originalConsoleError; + const errors = []; + ReactDOMClient.hydrateRoot(container, , { + onRecoverableError(error) { + errors.push(error.message); + }, + }); + expect(Scheduler).toFlushAndYield([]); + if (gate(flags => flags.enableFloat)) { + expect(errors).toEqual([]); + // with float, the title doesn't render on the client or on the server + expect(getVisibleChildren(container)).toEqual(); + } else { + expect(errors).toEqual( + [ + gate(flags => flags.enableClientRenderFallbackOnTextMismatch) + ? 'Text content does not match server-rendered HTML.' + : null, + 'Hydration failed because the initial UI does not match what was rendered on the server.', + 'There was an error while hydrating. Because the error happened outside of a Suspense boundary, the entire root will switch to client rendering.', + ].filter(Boolean), + ); + expect(getVisibleChildren(container)).toEqual( + <title>{['hello1', 'hello2']}, + ); } }); it('should warn in dev if you pass a React Component as a child to ', async () => { - const originalConsoleError = console.error; - const mockError = jest.fn(); - console.error = (...args) => { - if (args.length > 1) { - if (typeof args[1] === 'object') { - mockError(args[0].split('\n')[0]); - return; - } - } - mockError(...args.map(normalizeCodeLocInfo)); - }; - function IndirectTitle() { return 'hello'; } function App() { return ( - <title> - <IndirectTitle /> - + <> + + <IndirectTitle /> + + ); } - try { - prepareJSDOMForTitle(); + prepareJSDOMForTitle(); - await act(async () => { - const {pipe} = renderToPipeableStream(); - pipe(writable); - }); - if (__DEV__) { - expect(mockError).toHaveBeenCalledWith( - 'Warning: A title element received a React element for children. ' + - 'In the browser title Elements can only have Text Nodes as children. If ' + - 'the children being rendered output more than a single text node in aggregate the browser ' + - 'will display markup and comments as text in the title and hydration will likely fail and ' + - 'fall back to client rendering%s', - '\n' + ' in title (at **)\n' + ' in App (at **)', - ); - } else { - expect(mockError).not.toHaveBeenCalled(); - } + if (gate(flags => flags.enableFloat)) { + await expect(async () => { + await act(async () => { + const {pipe} = renderToPipeableStream(); + pipe(writable); + }); + }).toErrorDev([ + 'React expects the `children` prop of tags to be a string, number, or object with a novel `toString` method but found an object that appears to be a React element which never implements a suitable `toString` method. Browsers treat all child Nodes of <title> tags as Text content and React expects to be able to convert children of <title> tags to a single string value which is why rendering React elements is not supported. If the `children` of <title> is a React Component try moving the <title> tag into that component. If the `children` of <title> is some HTML markup change it to be Text only to be valid HTML.', + ]); + } else { + await expect(async () => { + await act(async () => { + const {pipe} = renderToPipeableStream(<App />); + pipe(writable); + }); + }).toErrorDev([ + 'A title element received a React element for children. In the browser title Elements can only have Text Nodes as children. If the children being rendered output more than a single text node in aggregate the browser will display markup and comments as text in the title and hydration will likely fail and fall back to client rendering', + ]); + } - if (gate(flags => flags.enableFloat)) { - // object titles are toStringed when float is on - expect(getVisibleChildren(container)).toEqual( - <title>{'[object Object]'}, - ); - } else { - expect(getVisibleChildren(container)).toEqual(hello); - } + if (gate(flags => flags.enableFloat)) { + // object titles are toStringed when float is on + expect(getVisibleChildren(container)).toEqual( + {'[object Object]'}, + ); + } else { + expect(getVisibleChildren(container)).toEqual(hello); + } - const errors = []; - ReactDOMClient.hydrateRoot(container, , { - onRecoverableError(error) { - errors.push(error.message); - }, - }); - expect(Scheduler).toFlushAndYield([]); - expect(errors).toEqual([]); - if (gate(flags => flags.enableFloat)) { - // object titles are toStringed when float is on - expect(getVisibleChildren(container)).toEqual( - {'[object Object]'}, - ); - } else { - expect(getVisibleChildren(container)).toEqual(hello); - } - } finally { - console.error = originalConsoleError; + const errors = []; + ReactDOMClient.hydrateRoot(container, , { + onRecoverableError(error) { + errors.push(error.message); + }, + }); + expect(Scheduler).toFlushAndYield([]); + expect(errors).toEqual([]); + if (gate(flags => flags.enableFloat)) { + // object titles are toStringed when float is on + expect(getVisibleChildren(container)).toEqual( + {'[object Object]'}, + ); + } else { + expect(getVisibleChildren(container)).toEqual(hello); } }); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js index 338e51c22474b..10f444593b9f6 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js @@ -62,9 +62,15 @@ describe('ReactDOMFizzServerBrowser', () => { , ); const result = await readResult(stream); - expect(result).toMatchInlineSnapshot( - `"hello world"`, - ); + if (gate(flags => flags.enableFloat)) { + expect(result).toMatchInlineSnapshot( + `"hello world"`, + ); + } else { + expect(result).toMatchInlineSnapshot( + `"hello world"`, + ); + } }); it('should emit bootstrap script src at the end', async () => { diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js index a5b2b5ac417a8..88c36d7a128a5 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js @@ -73,9 +73,16 @@ describe('ReactDOMFizzServerNode', () => { ); pipe(writable); jest.runAllTimers(); - expect(output.result).toMatchInlineSnapshot( - `"hello world"`, - ); + if (gate(flags => flags.enableFloat)) { + // with Float, we emit empty heads if they are elided when rendering + expect(output.result).toMatchInlineSnapshot( + `"hello world"`, + ); + } else { + expect(output.result).toMatchInlineSnapshot( + `"hello world"`, + ); + } }); it('should emit bootstrap script src at the end', () => { diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js index 43fe7ec09b78e..eed9a75753b35 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js @@ -64,9 +64,15 @@ describe('ReactDOMFizzStaticBrowser', () => { , ); const prelude = await readContent(result.prelude); - expect(prelude).toMatchInlineSnapshot( - `"hello world"`, - ); + if (gate(flags => flags.enableFloat)) { + expect(prelude).toMatchInlineSnapshot( + `"hello world"`, + ); + } else { + expect(prelude).toMatchInlineSnapshot( + `"hello world"`, + ); + } }); // @gate experimental diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js index 64b14ef793596..a419720ed72cd 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js @@ -65,9 +65,15 @@ describe('ReactDOMFizzStaticNode', () => { , ); const prelude = await readContent(result.prelude); - expect(prelude).toMatchInlineSnapshot( - `"hello world"`, - ); + if (gate(flags => flags.enableFloat)) { + expect(prelude).toMatchInlineSnapshot( + `"hello world"`, + ); + } else { + expect(prelude).toMatchInlineSnapshot( + `"hello world"`, + ); + } }); // @gate experimental diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index 97b4371ffe245..b2b1546ac4eff 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -623,6 +623,7 @@ describe('ReactDOMFloat', () => { ).toEqual(['']); }); + // @gate enableFloat it('can avoid inserting a late stylesheet if it already rendered on the client', async () => { await actIntoEmptyDocument(() => { renderToPipeableStream( @@ -689,10 +690,10 @@ describe('ReactDOMFloat', () => { resolveText('bar'); }); await act(() => { - let sheets = document.querySelectorAll( + const sheets = document.querySelectorAll( 'link[rel="stylesheet"][data-precedence]', ); - let event = document.createEvent('Event'); + const event = document.createEvent('Event'); event.initEvent('load', true, true); for (let i = 0; i < sheets.length; i++) { sheets[i].dispatchEvent(event); @@ -717,10 +718,10 @@ describe('ReactDOMFloat', () => { resolveText('foo'); }); await act(() => { - let sheets = document.querySelectorAll( + const sheets = document.querySelectorAll( 'link[rel="stylesheet"][data-precedence]', ); - let event = document.createEvent('Event'); + const event = document.createEvent('Event'); event.initEvent('load', true, true); for (let i = 0; i < sheets.length; i++) { sheets[i].dispatchEvent(event); @@ -743,6 +744,7 @@ describe('ReactDOMFloat', () => { ); }); + // @gate enableFloat it('can hoist and + +
second
+ + + + + + , + ).pipe(writable); + }); + + expect(getMeaningfulChildren(document)).toEqual( + + + + , + ); + + await act(() => { + resolveText('first'); + }); + + const styleTemplates = document.querySelectorAll( + 'template[data-precedence]', + ); + expect(styleTemplates.length).toBe(1); + expect(getMeaningfulChildren(styleTemplates[0].content)).toEqual( + , + ); + expect(getMeaningfulChildren(document)).toEqual( + + + + , + ); + + await act(() => { + resolveText('second'); + }); + + expect(getMeaningfulChildren(document)).toEqual( + + + + + + +
first
+
second
+ + , + ); + }); }); describe('Script Resources', () => { @@ -2608,6 +2701,7 @@ background-color: green; ); }); + // @gate enableFloat it('does not create script resources when inside an context', async () => { await actIntoEmptyDocument(() => { const {pipe} = renderToPipeableStream( @@ -2668,6 +2762,7 @@ background-color: green; ); }); + // @gate enableFloat it('does not create script resources when inside a