From 37c336cb4716d3a4cd850426594e0b690d4c3df8 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 9 Apr 2021 20:20:54 -0400 Subject: [PATCH 1/4] Add iterable support to Fizz --- packages/react-server/src/ReactFizzServer.js | 62 +++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 90053ceb02512..6f0303c4645fc 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -521,6 +521,8 @@ const didWarnAboutContextTypeOnFunctionComponent = {}; const didWarnAboutGetDerivedStateOnFunctionComponent = {}; let didWarnAboutReassigningProps = false; const didWarnAboutDefaultPropsOnFunctionComponent = {}; +let didWarnAboutGenerators = false; +let didWarnAboutMaps = false; // This would typically be a function component but we still support module pattern // components for some reason. @@ -708,6 +710,40 @@ function renderElement( } } +function validateIterable(iterable, iteratorFn: Function): void { + if (__DEV__) { + // We don't support rendering Generators because it's a mutation. + // See https://github.com/facebook/react/issues/12995 + if ( + typeof Symbol === 'function' && + // $FlowFixMe Flow doesn't know about toStringTag + iterable[Symbol.toStringTag] === 'Generator' + ) { + if (!didWarnAboutGenerators) { + console.error( + 'Using Generators as children is unsupported and will likely yield ' + + 'unexpected results because enumerating a generator mutates it. ' + + 'You may convert it to an array with `Array.from()` or the ' + + '`[...spread]` operator before rendering. Keep in mind ' + + 'you might need to polyfill these features for older browsers.', + ); + } + didWarnAboutGenerators = true; + } + + // Warn about using Maps as children + if ((iterable: any).entries === iteratorFn) { + if (!didWarnAboutMaps) { + console.error( + 'Using Maps as children is not supported. ' + + 'Use an array of keyed ReactElements instead.', + ); + } + didWarnAboutMaps = true; + } + } +} + // This function by it self renders a node and consumes the task by mutating it // to update the current execution state. function renderNodeDestructive( @@ -756,7 +792,30 @@ function renderNodeDestructive( const iteratorFn = getIteratorFn(node); if (iteratorFn) { - throw new Error('Not yet implemented node type.'); + if (__DEV__) { + validateIterable(node, iteratorFn()); + } + const iterator = iteratorFn.call(node); + if (iterator) { + let step = iterator.next(); + // If there are not entries, we need to push an empty so we start by checking that. + if (!step.done) { + do { + // Recursively render the rest. We need to use the non-destructive form + // so that we can safely pop back up and render the sibling if something + // suspends. + renderNode(request, task, step.value); + step = iterator.next(); + } while (!step.done); + return; + } + } + pushEmpty( + task.blockedSegment.chunks, + request.responseState, + task.assignID, + ); + task.assignID = null; } const childString = Object.prototype.toString.call(node); @@ -805,6 +864,7 @@ function renderNodeDestructive( // Any other type is assumed to be empty. pushEmpty(task.blockedSegment.chunks, request.responseState, task.assignID); + task.assignID = null; } function spawnNewSuspendedTask( From 9cb831563d7ef5686f0333fe92ec5cd533ff2c6f Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 9 Apr 2021 20:42:54 -0400 Subject: [PATCH 2/4] Support fragments and fragment like types --- packages/react-server/src/ReactFizzServer.js | 34 ++++++++++++++++++-- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 6f0303c4645fc..28883921669cd 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -62,6 +62,12 @@ import { REACT_PORTAL_TYPE, REACT_LAZY_TYPE, REACT_SUSPENSE_TYPE, + REACT_LEGACY_HIDDEN_TYPE, + REACT_DEBUG_TRACING_MODE_TYPE, + REACT_STRICT_MODE_TYPE, + REACT_PROFILER_TYPE, + REACT_SUSPENSE_LIST_TYPE, + REACT_FRAGMENT_TYPE, } from 'shared/ReactSymbols'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import { @@ -703,10 +709,32 @@ function renderElement( } } else if (typeof type === 'string') { renderHostElement(request, task, type, props); - } else if (type === REACT_SUSPENSE_TYPE) { - renderSuspenseBoundary(request, task, props); } else { - throw new Error('Not yet implemented element type.'); + switch (type) { + // TODO: LegacyHidden acts the same as a fragment. This only works + // because we currently assume that every instance of LegacyHidden is + // accompanied by a host component wrapper. In the hidden mode, the host + // component is given a `hidden` attribute, which ensures that the + // initial HTML is not visible. To support the use of LegacyHidden as a + // true fragment, without an extra DOM node, we would have to hide the + // initial HTML in some other way. + case REACT_LEGACY_HIDDEN_TYPE: + case REACT_DEBUG_TRACING_MODE_TYPE: + case REACT_STRICT_MODE_TYPE: + case REACT_PROFILER_TYPE: + case REACT_SUSPENSE_LIST_TYPE: // TODO: SuspenseList should control the boundaries. + case REACT_FRAGMENT_TYPE: { + renderNodeDestructive(request, task, props.children); + break; + } + case REACT_SUSPENSE_TYPE: { + renderSuspenseBoundary(request, task, props); + break; + } + default: { + throw new Error('Not yet implemented element type.'); + } + } } } From 909bd651d80165f1606d7789e305b02672c61dab Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 9 Apr 2021 20:43:09 -0400 Subject: [PATCH 3/4] Use some new types in tests --- .../src/__tests__/ReactDOMFizzServer-test.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 0b69bf72231a3..862c540c3b9f5 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -351,10 +351,12 @@ describe('ReactDOMFizzServer', () => { await act(async () => { const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable( }> - -
- -
+ <> + +
+ +
+ , writableA, { @@ -432,11 +434,11 @@ describe('ReactDOMFizzServer', () => { } function AsyncPath({id}) { - return {[]}; + return ; } function AsyncMi({id}) { - return {[]}; + return ; } function App() { @@ -601,7 +603,7 @@ describe('ReactDOMFizzServer', () => { // @gate experimental it('can stream into an SVG container', async () => { function AsyncPath({id}) { - return {[]}; + return ; } function App() { From 4edd83d647c2deab589d65d0b073b2a425b53e58 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 9 Apr 2021 20:47:38 -0400 Subject: [PATCH 4/4] Offscreen todo --- packages/react-server/src/ReactFizzServer.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 28883921669cd..3664e7ba48830 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -718,6 +718,7 @@ function renderElement( // initial HTML is not visible. To support the use of LegacyHidden as a // true fragment, without an extra DOM node, we would have to hide the // initial HTML in some other way. + // TODO: Add REACT_OFFSCREEN_TYPE here too with the same capability. case REACT_LEGACY_HIDDEN_TYPE: case REACT_DEBUG_TRACING_MODE_TYPE: case REACT_STRICT_MODE_TYPE: