From 09def5990b01d04433f852c9e6bc41039be052d9 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Wed, 26 Oct 2022 23:11:31 -0700 Subject: [PATCH] [Float] handle noscript context for Resources (#25559) stacked on https://github.com/facebook/react/pull/25569 On the client noscript already never renders children so no resources will be extracted from this context. On the server we now track if we are in a noscript context and turn off Resource semantics in this scope --- .../src/server/ReactDOMFloatServer.js | 4 + .../src/server/ReactDOMServerFormatConfig.js | 128 +++++++++++-- .../ReactDOMServerLegacyFormatConfig.js | 1 + .../src/__tests__/ReactDOMFloat-test.js | 174 ++++++++++++++++++ 4 files changed, 287 insertions(+), 20 deletions(-) diff --git a/packages/react-dom-bindings/src/server/ReactDOMFloatServer.js b/packages/react-dom-bindings/src/server/ReactDOMFloatServer.js index 46cfe75c95510..4af71458cb6a7 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMFloatServer.js +++ b/packages/react-dom-bindings/src/server/ReactDOMFloatServer.js @@ -863,6 +863,10 @@ export function resourcesFromLink(props: Props): boolean { } } if (props.onLoad || props.onError) { + // When a link has these props we can't treat it is a Resource but if we rendered it on the + // server it would look like a Resource in the rendered html (the onLoad/onError aren't emitted) + // Instead we expect the client to insert them rather than hydrate them which also guarantees + // that the onLoad and onError won't fire before the event handlers are attached return true; } diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js index 6efb10ff384ff..dd6350d2a743b 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js @@ -274,15 +274,18 @@ type InsertionMode = 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7; export type FormatContext = { insertionMode: InsertionMode, // root/svg/html/mathml/table selectedValue: null | string | Array, // the selected value(s) inside a + noscriptTagInScope: boolean, }; function createFormatContext( insertionMode: InsertionMode, selectedValue: null | string, + noscriptTagInScope: boolean, ): FormatContext { return { insertionMode, selectedValue, + noscriptTagInScope, }; } @@ -293,7 +296,7 @@ export function createRootFormatContext(namespaceURI?: string): FormatContext { : namespaceURI === 'http://www.w3.org/1998/Math/MathML' ? MATHML_MODE : ROOT_HTML_MODE; - return createFormatContext(insertionMode, null); + return createFormatContext(insertionMode, null, false); } export function getChildFormatContext( @@ -302,38 +305,77 @@ export function getChildFormatContext( props: Object, ): FormatContext { switch (type) { + case 'noscript': + return createFormatContext(HTML_MODE, null, true); case 'select': return createFormatContext( HTML_MODE, props.value != null ? props.value : props.defaultValue, + parentContext.noscriptTagInScope, ); case 'svg': - return createFormatContext(SVG_MODE, null); + return createFormatContext( + SVG_MODE, + null, + parentContext.noscriptTagInScope, + ); case 'math': - return createFormatContext(MATHML_MODE, null); + return createFormatContext( + MATHML_MODE, + null, + parentContext.noscriptTagInScope, + ); case 'foreignObject': - return createFormatContext(HTML_MODE, null); + return createFormatContext( + HTML_MODE, + null, + parentContext.noscriptTagInScope, + ); // Table parents are special in that their children can only be created at all if they're // wrapped in a table parent. So we need to encode that we're entering this mode. case 'table': - return createFormatContext(HTML_TABLE_MODE, null); + return createFormatContext( + HTML_TABLE_MODE, + null, + parentContext.noscriptTagInScope, + ); case 'thead': case 'tbody': case 'tfoot': - return createFormatContext(HTML_TABLE_BODY_MODE, null); + return createFormatContext( + HTML_TABLE_BODY_MODE, + null, + parentContext.noscriptTagInScope, + ); case 'colgroup': - return createFormatContext(HTML_COLGROUP_MODE, null); + return createFormatContext( + HTML_COLGROUP_MODE, + null, + parentContext.noscriptTagInScope, + ); case 'tr': - return createFormatContext(HTML_TABLE_ROW_MODE, null); + return createFormatContext( + HTML_TABLE_ROW_MODE, + null, + parentContext.noscriptTagInScope, + ); } if (parentContext.insertionMode >= HTML_TABLE_MODE) { // Whatever tag this was, it wasn't a table parent or other special parent, so we must have // entered plain HTML again. - return createFormatContext(HTML_MODE, null); + return createFormatContext( + HTML_MODE, + null, + parentContext.noscriptTagInScope, + ); } if (parentContext.insertionMode === ROOT_HTML_MODE) { // We've emitted the root and is now in plain HTML mode. - return createFormatContext(HTML_MODE, null); + return createFormatContext( + HTML_MODE, + null, + parentContext.noscriptTagInScope, + ); } return parentContext; } @@ -1155,8 +1197,13 @@ function pushBase( props: Object, responseState: ResponseState, textEmbedded: boolean, + noscriptTagInScope: boolean, ): ReactNodeList { - if (enableFloat && resourcesFromElement('base', props)) { + if ( + enableFloat && + !noscriptTagInScope && + resourcesFromElement('base', props) + ) { if (textEmbedded) { // This link follows text but we aren't writing a tag. while not as efficient as possible we need // to be safe and assume text will follow by inserting a textSeparator @@ -1175,8 +1222,13 @@ function pushMeta( props: Object, responseState: ResponseState, textEmbedded: boolean, + noscriptTagInScope: boolean, ): ReactNodeList { - if (enableFloat && resourcesFromElement('meta', props)) { + if ( + enableFloat && + !noscriptTagInScope && + resourcesFromElement('meta', props) + ) { if (textEmbedded) { // This link follows text but we aren't writing a tag. while not as efficient as possible we need // to be safe and assume text will follow by inserting a textSeparator @@ -1195,8 +1247,9 @@ function pushLink( props: Object, responseState: ResponseState, textEmbedded: boolean, + noscriptTagInScope: boolean, ): ReactNodeList { - if (enableFloat && resourcesFromLink(props)) { + if (enableFloat && !noscriptTagInScope && resourcesFromLink(props)) { if (textEmbedded) { // This link follows text but we aren't writing a tag. while not as efficient as possible we need // to be safe and assume text will follow by inserting a textSeparator @@ -1318,6 +1371,7 @@ function pushTitle( target: Array, props: Object, responseState: ResponseState, + noscriptTagInScope: boolean, ): ReactNodeList { if (__DEV__) { const children = props.children; @@ -1359,7 +1413,11 @@ function pushTitle( } } - if (enableFloat && resourcesFromElement('title', props)) { + if ( + enableFloat && + !noscriptTagInScope && + resourcesFromElement('title', props) + ) { // We have converted this link exclusively to a resource and no longer // need to emit it return null; @@ -1520,8 +1578,9 @@ function pushScript( props: Object, responseState: ResponseState, textEmbedded: boolean, + noscriptTagInScope: boolean, ): null { - if (enableFloat && resourcesFromScript(props)) { + if (enableFloat && !noscriptTagInScope && resourcesFromScript(props)) { if (textEmbedded) { // This link follows text but we aren't writing a tag. while not as efficient as possible we need // to be safe and assume text will follow by inserting a textSeparator @@ -1863,18 +1922,47 @@ export function pushStartInstance( return pushStartMenuItem(target, props, responseState); case 'title': return enableFloat - ? pushTitle(target, props, responseState) + ? pushTitle( + target, + props, + responseState, + formatContext.noscriptTagInScope, + ) : pushStartTitle(target, props, responseState); case 'link': - return pushLink(target, props, responseState, textEmbedded); + return pushLink( + target, + props, + responseState, + textEmbedded, + formatContext.noscriptTagInScope, + ); case 'script': return enableFloat - ? pushScript(target, props, responseState, textEmbedded) + ? pushScript( + target, + props, + responseState, + textEmbedded, + formatContext.noscriptTagInScope, + ) : pushStartGenericElement(target, props, type, responseState); case 'meta': - return pushMeta(target, props, responseState, textEmbedded); + return pushMeta( + target, + props, + responseState, + textEmbedded, + formatContext.noscriptTagInScope, + ); case 'base': - return pushBase(target, props, responseState, textEmbedded); + return pushBase( + target, + props, + responseState, + textEmbedded, + formatContext.noscriptTagInScope, + ); // Newline eating tags case 'listing': case 'pre': { diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerLegacyFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerLegacyFormatConfig.js index c8535ec521ba3..01e70273a656f 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerLegacyFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerLegacyFormatConfig.js @@ -72,6 +72,7 @@ export function createRootFormatContext(): FormatContext { return { insertionMode: HTML_MODE, // We skip the root mode because we don't want to emit the DOCTYPE in legacy mode. selectedValue: null, + noscriptTagInScope: false, }; } diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index 7c4e826b22f8c..276cac2b3e734 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -290,6 +290,9 @@ describe('ReactDOMFloat', () => { {}} /> foo +