From 87e900795debf38331ea375e165d1faf7d716cd6 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 4 Mar 2019 13:30:38 -0800 Subject: [PATCH 1/5] Convert ReactSuspensePlaceholder tests to use noop Instead of the test renderer, since test renderer does not support running in persistent mode. --- .../ReactSuspensePlaceholder-test.internal.js | 116 +++++++++++------- 1 file changed, 72 insertions(+), 44 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js index fc49bd158aafc..47d1349f41d14 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js @@ -21,7 +21,7 @@ runPlaceholderTests('ReactSuspensePlaceholder (persistence)', () => function runPlaceholderTests(suiteLabel, loadReactNoop) { let Profiler; let React; - let ReactTestRenderer; + let ReactNoop; let Scheduler; let ReactFeatureFlags; let ReactCache; @@ -38,7 +38,7 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { ReactFeatureFlags.enableProfilerTimer = true; ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; React = require('react'); - ReactTestRenderer = require('react-test-renderer'); + ReactNoop = require('react-noop-renderer'); Scheduler = require('scheduler'); ReactCache = require('react-cache'); @@ -134,9 +134,7 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { } // Initial mount - const root = ReactTestRenderer.create(, { - unstable_isConcurrent: true, - }); + ReactNoop.render(); expect(Scheduler).toFlushAndYield([ 'A', @@ -144,14 +142,14 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { 'C', 'Loading...', ]); - expect(root).toMatchRenderedOutput(null); + expect(ReactNoop).toMatchRenderedOutput(null); jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [B]']); expect(Scheduler).toFlushAndYield(['A', 'B', 'C']); - expect(root).toMatchRenderedOutput( + expect(ReactNoop).toMatchRenderedOutput( B @@ -160,13 +158,20 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { ); // Update - root.update(); + ReactNoop.render(); expect(Scheduler).toFlushAndYield(['Suspend! [B2]', 'C', 'Loading...']); // Time out the update jest.advanceTimersByTime(750); expect(Scheduler).toFlushAndYield([]); - expect(root).toMatchRenderedOutput('Loading...'); + expect(ReactNoop).toMatchRenderedOutput( + + + + + Loading... + , + ); // Resolve the promise jest.advanceTimersByTime(1000); @@ -175,7 +180,7 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { // Render the final update. A should still be hidden, because it was // given a `hidden` prop. - expect(root).toMatchRenderedOutput( + expect(ReactNoop).toMatchRenderedOutput( B2 @@ -196,9 +201,7 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { } // Initial mount - const root = ReactTestRenderer.create(, { - unstable_isConcurrent: true, - }); + ReactNoop.render(); expect(Scheduler).toFlushAndYield([ 'A', @@ -207,15 +210,15 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { 'Loading...', ]); - expect(root).toMatchRenderedOutput(null); + expect(ReactNoop).toMatchRenderedOutput(null); jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [B]']); expect(Scheduler).toFlushAndYield(['A', 'B', 'C']); - expect(root).toMatchRenderedOutput('ABC'); + expect(ReactNoop).toMatchRenderedOutput('ABC'); // Update - root.update(); + ReactNoop.render(); expect(Scheduler).toFlushAndYield([ 'A', 'Suspend! [B2]', @@ -225,7 +228,7 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { // Time out the update jest.advanceTimersByTime(750); expect(Scheduler).toFlushAndYield([]); - expect(root).toMatchRenderedOutput('Loading...'); + expect(ReactNoop).toMatchRenderedOutput('Loading...'); // Resolve the promise jest.advanceTimersByTime(1000); @@ -234,7 +237,7 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { // Render the final update. A should still be hidden, because it was // given a `hidden` prop. - expect(root).toMatchRenderedOutput('AB2C'); + expect(ReactNoop).toMatchRenderedOutput('AB2C'); }); describe('profiler durations', () => { @@ -272,8 +275,15 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { describe('when suspending during mount', () => { it('properly accounts for base durations when a suspended times out in a sync tree', () => { - const root = ReactTestRenderer.create(); - expect(root.toJSON()).toEqual('Loading...'); + ReactNoop.renderLegacySyncRoot(); + expect(Scheduler).toHaveYielded([ + 'App', + 'Suspending', + 'Suspend! [Loaded]', + 'Text', + 'Fallback', + ]); + expect(ReactNoop).toMatchRenderedOutput('Loading...'); expect(onRender).toHaveBeenCalledTimes(1); // Initial mount only shows the "Loading..." Fallback. @@ -284,7 +294,11 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { jest.advanceTimersByTime(1000); - expect(root.toJSON()).toEqual(['Loaded', 'Text']); + expect(Scheduler).toHaveYielded([ + 'Promise resolved [Loaded]', + 'Loaded', + ]); + expect(ReactNoop).toMatchRenderedOutput('LoadedText'); expect(onRender).toHaveBeenCalledTimes(2); // When the suspending data is resolved and our final UI is rendered, @@ -295,9 +309,7 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { }); it('properly accounts for base durations when a suspended times out in a concurrent tree', () => { - const root = ReactTestRenderer.create(, { - unstable_isConcurrent: true, - }); + ReactNoop.render(); expect(Scheduler).toFlushAndYield([ 'App', @@ -306,11 +318,11 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { 'Text', 'Fallback', ]); - expect(root).toMatchRenderedOutput(null); + expect(ReactNoop).toMatchRenderedOutput(null); // Show the fallback UI. jest.advanceTimersByTime(750); - expect(root).toMatchRenderedOutput('Loading...'); + expect(ReactNoop).toMatchRenderedOutput('Loading...'); expect(onRender).toHaveBeenCalledTimes(1); // Initial mount only shows the "Loading..." Fallback. @@ -323,7 +335,7 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { jest.advanceTimersByTime(250); expect(Scheduler).toHaveYielded(['Promise resolved [Loaded]']); expect(Scheduler).toFlushAndYield(['Suspending', 'Loaded', 'Text']); - expect(root).toMatchRenderedOutput('LoadedText'); + expect(ReactNoop).toMatchRenderedOutput('LoadedText'); expect(onRender).toHaveBeenCalledTimes(2); // When the suspending data is resolved and our final UI is rendered, @@ -335,10 +347,11 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { describe('when suspending during update', () => { it('properly accounts for base durations when a suspended times out in a sync tree', () => { - const root = ReactTestRenderer.create( + ReactNoop.renderLegacySyncRoot( , ); - expect(root.toJSON()).toEqual('Text'); + expect(Scheduler).toHaveYielded(['App', 'Text']); + expect(ReactNoop).toMatchRenderedOutput('Text'); expect(onRender).toHaveBeenCalledTimes(1); // Initial mount only shows the "Text" text. @@ -346,8 +359,15 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { expect(onRender.mock.calls[0][2]).toBe(5); expect(onRender.mock.calls[0][3]).toBe(5); - root.update(); - expect(root.toJSON()).toEqual('Loading...'); + ReactNoop.render(); + expect(Scheduler).toHaveYielded([ + 'App', + 'Suspending', + 'Suspend! [Loaded]', + 'Text', + 'Fallback', + ]); + expect(ReactNoop).toMatchRenderedOutput('Loading...'); expect(onRender).toHaveBeenCalledTimes(2); // The suspense update should only show the "Loading..." Fallback. @@ -356,10 +376,17 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { expect(onRender.mock.calls[1][2]).toBe(18); expect(onRender.mock.calls[1][3]).toBe(18); - root.update( + ReactNoop.renderLegacySyncRoot( , ); - expect(root.toJSON()).toEqual('Loading...'); + expect(Scheduler).toHaveYielded([ + 'App', + 'Suspending', + 'Suspend! [Loaded]', + 'New', + 'Fallback', + ]); + expect(ReactNoop).toMatchRenderedOutput('Loading...'); expect(onRender).toHaveBeenCalledTimes(3); // If we force another update while still timed out, @@ -370,7 +397,11 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { jest.advanceTimersByTime(1000); - expect(root.toJSON()).toEqual(['Loaded', 'New']); + expect(Scheduler).toHaveYielded([ + 'Promise resolved [Loaded]', + 'Loaded', + ]); + expect(ReactNoop).toMatchRenderedOutput('LoadedNew'); expect(onRender).toHaveBeenCalledTimes(4); // When the suspending data is resolved and our final UI is rendered, @@ -381,15 +412,12 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { }); it('properly accounts for base durations when a suspended times out in a concurrent tree', () => { - const root = ReactTestRenderer.create( + ReactNoop.render( , - { - unstable_isConcurrent: true, - }, ); expect(Scheduler).toFlushAndYield(['App', 'Text']); - expect(root).toMatchRenderedOutput('Text'); + expect(ReactNoop).toMatchRenderedOutput('Text'); expect(onRender).toHaveBeenCalledTimes(1); // Initial mount only shows the "Text" text. @@ -397,7 +425,7 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { expect(onRender.mock.calls[0][2]).toBe(5); expect(onRender.mock.calls[0][3]).toBe(5); - root.update(); + ReactNoop.render(); expect(Scheduler).toFlushAndYield([ 'App', 'Suspending', @@ -405,11 +433,11 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { 'Text', 'Fallback', ]); - expect(root).toMatchRenderedOutput('Text'); + expect(ReactNoop).toMatchRenderedOutput('Text'); // Show the fallback UI. jest.advanceTimersByTime(750); - expect(root).toMatchRenderedOutput('Loading...'); + expect(ReactNoop).toMatchRenderedOutput('Loading...'); expect(onRender).toHaveBeenCalledTimes(2); // The suspense update should only show the "Loading..." Fallback. @@ -421,7 +449,7 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { expect(onRender.mock.calls[1][3]).toBe(15); // Update again while timed out. - root.update( + ReactNoop.render( , ); expect(Scheduler).toFlushAndYield([ @@ -431,7 +459,7 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { 'New', 'Fallback', ]); - expect(root).toMatchRenderedOutput('Loading...'); + expect(ReactNoop).toMatchRenderedOutput('Loading...'); expect(onRender).toHaveBeenCalledTimes(2); // Resolve the pending promise. From 60c899b3c5420d3ca7190ed6597829b67bffc09f Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 4 Mar 2019 16:16:39 -0800 Subject: [PATCH 2/5] Run Placeholder tests in persistent mode, too --- .../src/ReactFiberCommitWork.js | 98 ++++++++++--------- .../src/ReactFiberCompleteWork.js | 70 ++++++------- .../ReactSuspensePlaceholder-test.internal.js | 11 +-- 3 files changed, 94 insertions(+), 85 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 13e79828a92be..14345d3a30ad6 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -1131,6 +1131,13 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void { commitHookEffectList(UnmountMutation, MountMutation, finishedWork); return; } + case Profiler: { + return; + } + case SuspenseComponent: { + commitSuspenseComponent(finishedWork); + return; + } } commitContainer(finishedWork); @@ -1199,50 +1206,7 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void { return; } case SuspenseComponent: { - let newState: SuspenseState | null = finishedWork.memoizedState; - - let newDidTimeout; - let primaryChildParent = finishedWork; - if (newState === null) { - newDidTimeout = false; - } else { - newDidTimeout = true; - primaryChildParent = finishedWork.child; - if (newState.timedOutAt === NoWork) { - // If the children had not already timed out, record the time. - // This is used to compute the elapsed time during subsequent - // attempts to render the children. - newState.timedOutAt = requestCurrentTime(); - } - } - - if (primaryChildParent !== null) { - hideOrUnhideAllChildren(primaryChildParent, newDidTimeout); - } - - // If this boundary just timed out, then it will have a set of thenables. - // For each thenable, attach a listener so that when it resolves, React - // attempts to re-render the boundary in the primary (pre-timeout) state. - const thenables: Set | null = (finishedWork.updateQueue: any); - if (thenables !== null) { - finishedWork.updateQueue = null; - let retryCache = finishedWork.stateNode; - if (retryCache === null) { - retryCache = finishedWork.stateNode = new PossiblyWeakSet(); - } - thenables.forEach(thenable => { - // Memoize using the boundary fiber to prevent redundant listeners. - let retry = resolveRetryThenable.bind(null, finishedWork, thenable); - if (enableSchedulerTracing) { - retry = Schedule_tracing_wrap(retry); - } - if (!retryCache.has(thenable)) { - retryCache.add(thenable); - thenable.then(retry, retry); - } - }); - } - + commitSuspenseComponent(finishedWork); return; } case IncompleteClassComponent: { @@ -1258,6 +1222,52 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void { } } +function commitSuspenseComponent(finishedWork: Fiber) { + let newState: SuspenseState | null = finishedWork.memoizedState; + + let newDidTimeout; + let primaryChildParent = finishedWork; + if (newState === null) { + newDidTimeout = false; + } else { + newDidTimeout = true; + primaryChildParent = finishedWork.child; + if (newState.timedOutAt === NoWork) { + // If the children had not already timed out, record the time. + // This is used to compute the elapsed time during subsequent + // attempts to render the children. + newState.timedOutAt = requestCurrentTime(); + } + } + + if (supportsMutation && primaryChildParent !== null) { + hideOrUnhideAllChildren(primaryChildParent, newDidTimeout); + } + + // If this boundary just timed out, then it will have a set of thenables. + // For each thenable, attach a listener so that when it resolves, React + // attempts to re-render the boundary in the primary (pre-timeout) state. + const thenables: Set | null = (finishedWork.updateQueue: any); + if (thenables !== null) { + finishedWork.updateQueue = null; + let retryCache = finishedWork.stateNode; + if (retryCache === null) { + retryCache = finishedWork.stateNode = new PossiblyWeakSet(); + } + thenables.forEach(thenable => { + // Memoize using the boundary fiber to prevent redundant listeners. + let retry = resolveRetryThenable.bind(null, finishedWork, thenable); + if (enableSchedulerTracing) { + retry = Schedule_tracing_wrap(retry); + } + if (!retryCache.has(thenable)) { + retryCache.add(thenable); + thenable.then(retry, retry); + } + }); + } +} + function commitResetTextContent(current: Fiber) { if (!supportsMutation) { return; diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 86d3ec7d86454..5d68823ca4e91 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -253,20 +253,17 @@ if (supportsMutation) { // down its children. Instead, we'll get insertions from each child in // the portal directly. } else if (node.tag === SuspenseComponent) { - const current = node.alternate; - if (current !== null) { - const oldState: SuspenseState = current.memoizedState; - const newState: SuspenseState = node.memoizedState; - const oldIsHidden = oldState !== null; - const newIsHidden = newState !== null; - if (oldIsHidden !== newIsHidden) { - // The placeholder either just timed out or switched back to the normal - // children after having previously timed out. Toggle the visibility of - // the direct host children. - const primaryChildParent = newIsHidden ? node.child : node; - if (primaryChildParent !== null) { - appendAllChildren(parent, primaryChildParent, true, newIsHidden); - } + if ((node.effectTag & Update) !== NoEffect) { + // Need to toggle the visibility of the primary children. + const newIsHidden = node.memoizedState !== null; + if (newIsHidden) { + const primaryChildParent = node.child; + appendAllChildren(parent, primaryChildParent, true, newIsHidden); + node = primaryChildParent.sibling; + continue; + } else { + const primaryChildParent = node; + appendAllChildren(parent, primaryChildParent, true, newIsHidden); // eslint-disable-next-line no-labels break branches; } @@ -356,25 +353,27 @@ if (supportsMutation) { // down its children. Instead, we'll get insertions from each child in // the portal directly. } else if (node.tag === SuspenseComponent) { - const current = node.alternate; - if (current !== null) { - const oldState: SuspenseState = current.memoizedState; - const newState: SuspenseState = node.memoizedState; - const oldIsHidden = oldState !== null; - const newIsHidden = newState !== null; - if (oldIsHidden !== newIsHidden) { - // The placeholder either just timed out or switched back to the normal - // children after having previously timed out. Toggle the visibility of - // the direct host children. - const primaryChildParent = newIsHidden ? node.child : node; - if (primaryChildParent !== null) { - appendAllChildrenToContainer( - containerChildSet, - primaryChildParent, - true, - newIsHidden, - ); - } + if ((node.effectTag & Update) !== NoEffect) { + // Need to toggle the visibility of the primary children. + const newIsHidden = node.memoizedState !== null; + if (newIsHidden) { + const primaryChildParent = node.child; + appendAllChildrenToContainer( + containerChildSet, + primaryChildParent, + true, + newIsHidden, + ); + node = primaryChildParent.sibling; + continue; + } else { + const primaryChildParent = node; + appendAllChildrenToContainer( + containerChildSet, + primaryChildParent, + true, + newIsHidden, + ); // eslint-disable-next-line no-labels break branches; } @@ -562,6 +561,10 @@ function completeWork( break; } case HostRoot: { + // In persistent mode, updateHostContainer may need to create new + // instances, so this needs to happen before we push/pop the contexts. + updateHostContainer(workInProgress); + popHostContainer(workInProgress); popTopLevelLegacyContextObject(workInProgress); const fiberRoot = (workInProgress.stateNode: FiberRoot); @@ -577,7 +580,6 @@ function completeWork( // TODO: Delete this when we delete isMounted and findDOMNode. workInProgress.effectTag &= ~Placement; } - updateHostContainer(workInProgress); break; } case HostComponent: { diff --git a/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js index 47d1349f41d14..a531cde168af9 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js @@ -8,12 +8,9 @@ * @jest-environment node */ -// TODO: This does nothing since it was migrated from noop renderer to test -// renderer! Switch back to noop renderer, or add persistent mode to test -// renderer, or merge the two renderers into one somehow. -// runPlaceholderTests('ReactSuspensePlaceholder (mutation)', () => -// require('react-noop-renderer'), -// ); +runPlaceholderTests('ReactSuspensePlaceholder (mutation)', () => + require('react-noop-renderer'), +); runPlaceholderTests('ReactSuspensePlaceholder (persistence)', () => require('react-noop-renderer/persistent'), ); @@ -38,7 +35,7 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { ReactFeatureFlags.enableProfilerTimer = true; ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; React = require('react'); - ReactNoop = require('react-noop-renderer'); + ReactNoop = loadReactNoop(); Scheduler = require('scheduler'); ReactCache = require('react-cache'); From bbf38288828e8fa1980cd1fd5683e589a7941a61 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 4 Mar 2019 16:50:14 -0800 Subject: [PATCH 3/5] Fix Flow and lint --- .../src/ReactFiberCompleteWork.js | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 5d68823ca4e91..d3f7625d5c21a 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -17,7 +17,6 @@ import type { Container, ChildSet, } from './ReactFiberHostConfig'; -import type {SuspenseState} from './ReactFiberSuspenseComponent'; import { IndeterminateComponent, @@ -258,9 +257,11 @@ if (supportsMutation) { const newIsHidden = node.memoizedState !== null; if (newIsHidden) { const primaryChildParent = node.child; - appendAllChildren(parent, primaryChildParent, true, newIsHidden); - node = primaryChildParent.sibling; - continue; + if (primaryChildParent !== null) { + appendAllChildren(parent, primaryChildParent, true, newIsHidden); + node = primaryChildParent.sibling; + continue; + } } else { const primaryChildParent = node; appendAllChildren(parent, primaryChildParent, true, newIsHidden); @@ -358,14 +359,16 @@ if (supportsMutation) { const newIsHidden = node.memoizedState !== null; if (newIsHidden) { const primaryChildParent = node.child; - appendAllChildrenToContainer( - containerChildSet, - primaryChildParent, - true, - newIsHidden, - ); - node = primaryChildParent.sibling; - continue; + if (primaryChildParent !== null) { + appendAllChildrenToContainer( + containerChildSet, + primaryChildParent, + true, + newIsHidden, + ); + node = primaryChildParent.sibling; + continue; + } } else { const primaryChildParent = node; appendAllChildrenToContainer( From cb6e87ddbd4ad7cc06b721b189dcb95cfc012f54 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 4 Mar 2019 18:17:53 -0800 Subject: [PATCH 4/5] Hidden text instances should have correct host context Adds a test for a subtle edge case that only occurs in persistent mode. --- .../src/createReactNoop.js | 76 ++++++++++-- .../src/ReactFiberCompleteWork.js | 113 ++++++++++++++---- .../ReactSuspensePlaceholder-test.internal.js | 57 +++++++++ 3 files changed, 215 insertions(+), 31 deletions(-) diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index b24bb68a2a8d1..157a66a82b3be 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -41,10 +41,18 @@ type Instance = {| text: string | null, prop: any, hidden: boolean, + context: HostContext, |}; -type TextInstance = {|text: string, id: number, hidden: boolean|}; +type TextInstance = {| + text: string, + id: number, + hidden: boolean, + context: HostContext, +|}; +type HostContext = Object; const NO_CONTEXT = {}; +const UPPERCASE_CONTEXT = {}; const UPDATE_SIGNAL = {}; if (__DEV__) { Object.freeze(NO_CONTEXT); @@ -190,10 +198,11 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { type: type, children: keepChildren ? instance.children : [], text: shouldSetTextContent(type, newProps) - ? (newProps.children: any) + '' + ? computeText((newProps.children: any) + '', instance.context) : null, prop: newProps.prop, hidden: newProps.hidden === true, + context: instance.context, }; Object.defineProperty(clone, 'id', { value: clone.id, @@ -203,6 +212,10 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { value: clone.text, enumerable: false, }); + Object.defineProperty(clone, 'context', { + value: clone.context, + enumerable: false, + }); hostCloneCounter++; return clone; } @@ -216,12 +229,23 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { ); } + function computeText(rawText, hostContext) { + return hostContext === UPPERCASE_CONTEXT ? rawText.toUpperCase() : rawText; + } + const sharedHostConfig = { getRootHostContext() { return NO_CONTEXT; }, - getChildHostContext() { + getChildHostContext( + parentHostContext: HostContext, + type: string, + rootcontainerInstance: Container, + ) { + if (type === 'uppercase') { + return UPPERCASE_CONTEXT; + } return NO_CONTEXT; }, @@ -229,7 +253,12 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { return instance; }, - createInstance(type: string, props: Props): Instance { + createInstance( + type: string, + props: Props, + rootContainerInstance: Container, + hostContext: HostContext, + ): Instance { if (type === 'errorInCompletePhase') { throw new Error('Error in host config.'); } @@ -238,10 +267,11 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { type: type, children: [], text: shouldSetTextContent(type, props) - ? (props.children: any) + '' + ? computeText((props.children: any) + '', hostContext) : null, prop: props.prop, hidden: props.hidden === true, + context: hostContext, }; // Hide from unit tests Object.defineProperty(inst, 'id', {value: inst.id, enumerable: false}); @@ -249,6 +279,10 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { value: inst.text, enumerable: false, }); + Object.defineProperty(inst, 'context', { + value: inst.context, + enumerable: false, + }); return inst; }, @@ -298,9 +332,21 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { hostContext: Object, internalInstanceHandle: Object, ): TextInstance { - const inst = {text: text, id: instanceCounter++, hidden: false}; + if (hostContext === UPPERCASE_CONTEXT) { + text = text.toUpperCase(); + } + const inst = { + text: text, + id: instanceCounter++, + hidden: false, + context: hostContext, + }; // Hide from unit tests Object.defineProperty(inst, 'id', {value: inst.id, enumerable: false}); + Object.defineProperty(inst, 'context', { + value: inst.context, + enumerable: false, + }); return inst; }, @@ -343,7 +389,10 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { instance.prop = newProps.prop; instance.hidden = newProps.hidden === true; if (shouldSetTextContent(type, newProps)) { - instance.text = (newProps.children: any) + ''; + instance.text = computeText( + (newProps.children: any) + '', + instance.context, + ); } }, @@ -353,7 +402,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { newText: string, ): void { hostUpdateCounter++; - textInstance.text = newText; + textInstance.text = computeText(newText, textInstance.context); }, appendChild, @@ -463,12 +512,21 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { hostContext: Object, internalInstanceHandle: Object, ): TextInstance { - const inst = {text: text, id: instanceCounter++, hidden: true}; + const inst = { + text: text, + id: instanceCounter++, + hidden: true, + context: hostContext, + }; // Hide from unit tests Object.defineProperty(inst, 'id', { value: inst.id, enumerable: false, }); + Object.defineProperty(inst, 'context', { + value: inst.context, + enumerable: false, + }); return inst; }, }; diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index d3f7625d5c21a..80a280afbb6d5 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -16,6 +16,7 @@ import type { Props, Container, ChildSet, + HostContext, } from './ReactFiberHostConfig'; import { @@ -50,6 +51,7 @@ import { import invariant from 'shared/invariant'; import { + getRootHostContext, createInstance, createTextInstance, createHiddenTextInstance, @@ -105,6 +107,8 @@ if (supportsMutation) { appendAllChildren = function( parent: Instance, workInProgress: Fiber, + rootContainerInstance: Container, + childHostContext: HostContext, needsVisibilityToggle: boolean, isHidden: boolean, ) { @@ -137,7 +141,11 @@ if (supportsMutation) { } }; - updateHostContainer = function(workInProgress: Fiber) { + updateHostContainer = function( + workInProgress: Fiber, + rootContainerInstance: Container, + childHostContext: HostContext, + ) { // Noop }; updateHostComponent = function( @@ -146,6 +154,7 @@ if (supportsMutation) { type: Type, newProps: Props, rootContainerInstance: Container, + childHostContext: HostContext, ) { // If we have an alternate, that means this is an update and we need to // schedule a side-effect to do the updates. @@ -198,6 +207,8 @@ if (supportsMutation) { appendAllChildren = function( parent: Instance, workInProgress: Fiber, + rootContainerInstance: Container, + childHostContext: HostContext, needsVisibilityToggle: boolean, isHidden: boolean, ) { @@ -227,20 +238,18 @@ if (supportsMutation) { let instance = node.stateNode; if (needsVisibilityToggle) { const text = node.memoizedProps; - const rootContainerInstance = getRootHostContainer(); - const currentHostContext = getHostContext(); if (isHidden) { instance = createHiddenTextInstance( text, rootContainerInstance, - currentHostContext, + childHostContext, workInProgress, ); } else { instance = createTextInstance( text, rootContainerInstance, - currentHostContext, + childHostContext, workInProgress, ); } @@ -258,13 +267,27 @@ if (supportsMutation) { if (newIsHidden) { const primaryChildParent = node.child; if (primaryChildParent !== null) { - appendAllChildren(parent, primaryChildParent, true, newIsHidden); + appendAllChildren( + parent, + primaryChildParent, + rootContainerInstance, + childHostContext, + true, + newIsHidden, + ); node = primaryChildParent.sibling; continue; } } else { const primaryChildParent = node; - appendAllChildren(parent, primaryChildParent, true, newIsHidden); + appendAllChildren( + parent, + primaryChildParent, + rootContainerInstance, + childHostContext, + true, + newIsHidden, + ); // eslint-disable-next-line no-labels break branches; } @@ -300,6 +323,8 @@ if (supportsMutation) { const appendAllChildrenToContainer = function( containerChildSet: ChildSet, workInProgress: Fiber, + rootContainerInstance: Container, + rootHostContext: HostContext, needsVisibilityToggle: boolean, isHidden: boolean, ) { @@ -329,20 +354,18 @@ if (supportsMutation) { let instance = node.stateNode; if (needsVisibilityToggle) { const text = node.memoizedProps; - const rootContainerInstance = getRootHostContainer(); - const currentHostContext = getHostContext(); if (isHidden) { instance = createHiddenTextInstance( text, rootContainerInstance, - currentHostContext, + rootHostContext, workInProgress, ); } else { instance = createTextInstance( text, rootContainerInstance, - currentHostContext, + rootHostContext, workInProgress, ); } @@ -363,6 +386,8 @@ if (supportsMutation) { appendAllChildrenToContainer( containerChildSet, primaryChildParent, + rootContainerInstance, + rootHostContext, true, newIsHidden, ); @@ -374,6 +399,8 @@ if (supportsMutation) { appendAllChildrenToContainer( containerChildSet, primaryChildParent, + rootContainerInstance, + rootHostContext, true, newIsHidden, ); @@ -407,7 +434,11 @@ if (supportsMutation) { node = node.sibling; } }; - updateHostContainer = function(workInProgress: Fiber) { + updateHostContainer = function( + workInProgress: Fiber, + rootContainerInstance: Container, + rootHostContext: HostContext, + ) { const portalOrRoot: { containerInfo: Container, pendingChildren: ChildSet, @@ -420,7 +451,14 @@ if (supportsMutation) { const container = portalOrRoot.containerInfo; let newChildSet = createContainerChildSet(container); // If children might have changed, we have to add them all to the set. - appendAllChildrenToContainer(newChildSet, workInProgress, false, false); + appendAllChildrenToContainer( + newChildSet, + workInProgress, + rootContainerInstance, + rootHostContext, + false, + false, + ); portalOrRoot.pendingChildren = newChildSet; // Schedule an update on the container to swap out the container. markUpdate(workInProgress); @@ -433,6 +471,7 @@ if (supportsMutation) { type: Type, newProps: Props, rootContainerInstance: Container, + childHostContext: HostContext, ) { const currentInstance = current.stateNode; const oldProps = current.memoizedProps; @@ -493,7 +532,14 @@ if (supportsMutation) { markUpdate(workInProgress); } else { // If children might have changed, we have to add them all to the set. - appendAllChildren(newInstance, workInProgress, false, false); + appendAllChildren( + newInstance, + workInProgress, + rootContainerInstance, + childHostContext, + false, + false, + ); } }; updateHostText = function( @@ -519,7 +565,11 @@ if (supportsMutation) { }; } else { // No host operations - updateHostContainer = function(workInProgress: Fiber) { + updateHostContainer = function( + workInProgress: Fiber, + rootContainerInstance: Container, + hostContext: HostContext, + ) { // Noop }; updateHostComponent = function( @@ -528,6 +578,7 @@ if (supportsMutation) { type: Type, newProps: Props, rootContainerInstance: Container, + childHostContext: HostContext, ) { // Noop }; @@ -564,10 +615,6 @@ function completeWork( break; } case HostRoot: { - // In persistent mode, updateHostContainer may need to create new - // instances, so this needs to happen before we push/pop the contexts. - updateHostContainer(workInProgress); - popHostContainer(workInProgress); popTopLevelLegacyContextObject(workInProgress); const fiberRoot = (workInProgress.stateNode: FiberRoot); @@ -583,11 +630,19 @@ function completeWork( // TODO: Delete this when we delete isMounted and findDOMNode. workInProgress.effectTag &= ~Placement; } + const rootContainerInstance = fiberRoot.containerInfo; + const rootHostContext = getRootHostContext(rootContainerInstance); + updateHostContainer( + workInProgress, + rootContainerInstance, + rootHostContext, + ); break; } case HostComponent: { - popHostContext(workInProgress); const rootContainerInstance = getRootHostContainer(); + const childHostContext = getHostContext(); + popHostContext(workInProgress); const type = workInProgress.type; if (current !== null && workInProgress.stateNode != null) { updateHostComponent( @@ -596,6 +651,7 @@ function completeWork( type, newProps, rootContainerInstance, + childHostContext, ); if (current.ref !== workInProgress.ref) { @@ -641,7 +697,14 @@ function completeWork( workInProgress, ); - appendAllChildren(instance, workInProgress, false, false); + appendAllChildren( + instance, + workInProgress, + rootContainerInstance, + childHostContext, + false, + false, + ); // Certain renderers require commit-time effects for initial mount. // (eg DOM renderer supports auto-focus for certain elements). @@ -755,8 +818,14 @@ function completeWork( case Profiler: break; case HostPortal: + const rootContainerInstance = getRootHostContainer(); + const childHostContext = getHostContext(); popHostContainer(workInProgress); - updateHostContainer(workInProgress); + updateHostContainer( + workInProgress, + rootContainerInstance, + childHostContext, + ); break; case ContextProvider: // Pop provider fiber diff --git a/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js index a531cde168af9..637779adbd7d8 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js @@ -237,6 +237,63 @@ function runPlaceholderTests(suiteLabel, loadReactNoop) { expect(ReactNoop).toMatchRenderedOutput('AB2C'); }); + it('preserves host context for text nodes', () => { + function App(props) { + return ( + // uppercase is a special type that causes React Noop to render child + // text nodes as uppercase. + + }> + + + + + + ); + } + + // Initial mount + ReactNoop.render(); + + expect(Scheduler).toFlushAndYield([ + 'a', + 'Suspend! [b]', + 'c', + 'Loading...', + ]); + + expect(ReactNoop).toMatchRenderedOutput(null); + + jest.advanceTimersByTime(1000); + expect(Scheduler).toHaveYielded(['Promise resolved [b]']); + expect(Scheduler).toFlushAndYield(['a', 'b', 'c']); + expect(ReactNoop).toMatchRenderedOutput(ABC); + + // Update + ReactNoop.render(); + expect(Scheduler).toFlushAndYield([ + 'a', + 'Suspend! [b2]', + 'c', + 'Loading...', + ]); + // Time out the update + jest.advanceTimersByTime(750); + expect(Scheduler).toFlushAndYield([]); + expect(ReactNoop).toMatchRenderedOutput( + LOADING..., + ); + + // Resolve the promise + jest.advanceTimersByTime(1000); + expect(Scheduler).toHaveYielded(['Promise resolved [b2]']); + expect(Scheduler).toFlushAndYield(['a', 'b2', 'c']); + + // Render the final update. A should still be hidden, because it was + // given a `hidden` prop. + expect(ReactNoop).toMatchRenderedOutput(AB2C); + }); + describe('profiler durations', () => { let App; let onRender; From 9182e166cb0c656b67ab42c228cacc54e81cd448 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 5 Mar 2019 15:32:26 -0800 Subject: [PATCH 5/5] createHiddenTextInstance -> cloneHiddenTextInstance This sidesteps the problem where createHiddenTextInstance needs access to the host context. --- .../src/ReactFabricHostConfig.js | 13 +- .../src/createReactNoop.js | 46 +++++-- .../src/ReactFiberCompleteWork.js | 129 +++--------------- .../src/forks/ReactFiberHostConfig.custom.js | 4 +- .../shared/HostConfigWithNoPersistence.js | 3 +- 5 files changed, 66 insertions(+), 129 deletions(-) diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index bb4646b190bce..0eed32d066246 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -406,10 +406,17 @@ export function cloneUnhiddenInstance( }; } -export function createHiddenTextInstance( +export function cloneHiddenTextInstance( + instance: Instance, + text: string, + internalInstanceHandle: Object, +): TextInstance { + throw new Error('Not yet implemented.'); +} + +export function cloneUnhiddenTextInstance( + instance: Instance, text: string, - rootContainerInstance: Container, - hostContext: HostContext, internalInstanceHandle: Object, ): TextInstance { throw new Error('Not yet implemented.'); diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 157a66a82b3be..62f993247cc96 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -502,32 +502,54 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { true, null, ); - clone.hidden = props.hidden; + clone.hidden = props.hidden === true; return clone; }, - createHiddenTextInstance( + cloneHiddenTextInstance( + instance: TextInstance, text: string, - rootContainerInstance: Container, - hostContext: Object, internalInstanceHandle: Object, ): TextInstance { - const inst = { - text: text, + const clone = { + text: instance.text, id: instanceCounter++, hidden: true, - context: hostContext, + context: instance.context, }; // Hide from unit tests - Object.defineProperty(inst, 'id', { - value: inst.id, + Object.defineProperty(clone, 'id', { + value: clone.id, enumerable: false, }); - Object.defineProperty(inst, 'context', { - value: inst.context, + Object.defineProperty(clone, 'context', { + value: clone.context, enumerable: false, }); - return inst; + return clone; + }, + + cloneUnhiddenTextInstance( + instance: TextInstance, + text: string, + internalInstanceHandle: Object, + ): TextInstance { + const clone = { + text: instance.text, + id: instanceCounter++, + hidden: false, + context: instance.context, + }; + // Hide from unit tests + Object.defineProperty(clone, 'id', { + value: clone.id, + enumerable: false, + }); + Object.defineProperty(clone, 'context', { + value: clone.context, + enumerable: false, + }); + return clone; }, }; diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 80a280afbb6d5..b1f374bb90bc6 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -16,7 +16,6 @@ import type { Props, Container, ChildSet, - HostContext, } from './ReactFiberHostConfig'; import { @@ -51,10 +50,8 @@ import { import invariant from 'shared/invariant'; import { - getRootHostContext, createInstance, createTextInstance, - createHiddenTextInstance, appendInitialChild, finalizeInitialChildren, prepareUpdate, @@ -63,6 +60,8 @@ import { cloneInstance, cloneHiddenInstance, cloneUnhiddenInstance, + cloneHiddenTextInstance, + cloneUnhiddenTextInstance, createContainerChildSet, appendChildToContainerChildSet, finalizeContainerChildren, @@ -107,8 +106,6 @@ if (supportsMutation) { appendAllChildren = function( parent: Instance, workInProgress: Fiber, - rootContainerInstance: Container, - childHostContext: HostContext, needsVisibilityToggle: boolean, isHidden: boolean, ) { @@ -141,11 +138,7 @@ if (supportsMutation) { } }; - updateHostContainer = function( - workInProgress: Fiber, - rootContainerInstance: Container, - childHostContext: HostContext, - ) { + updateHostContainer = function(workInProgress: Fiber) { // Noop }; updateHostComponent = function( @@ -154,7 +147,6 @@ if (supportsMutation) { type: Type, newProps: Props, rootContainerInstance: Container, - childHostContext: HostContext, ) { // If we have an alternate, that means this is an update and we need to // schedule a side-effect to do the updates. @@ -207,8 +199,6 @@ if (supportsMutation) { appendAllChildren = function( parent: Instance, workInProgress: Fiber, - rootContainerInstance: Container, - childHostContext: HostContext, needsVisibilityToggle: boolean, isHidden: boolean, ) { @@ -239,19 +229,9 @@ if (supportsMutation) { if (needsVisibilityToggle) { const text = node.memoizedProps; if (isHidden) { - instance = createHiddenTextInstance( - text, - rootContainerInstance, - childHostContext, - workInProgress, - ); + instance = cloneHiddenTextInstance(instance, text, node); } else { - instance = createTextInstance( - text, - rootContainerInstance, - childHostContext, - workInProgress, - ); + instance = cloneUnhiddenTextInstance(instance, text, node); } node.stateNode = instance; } @@ -267,27 +247,13 @@ if (supportsMutation) { if (newIsHidden) { const primaryChildParent = node.child; if (primaryChildParent !== null) { - appendAllChildren( - parent, - primaryChildParent, - rootContainerInstance, - childHostContext, - true, - newIsHidden, - ); + appendAllChildren(parent, primaryChildParent, true, newIsHidden); node = primaryChildParent.sibling; continue; } } else { const primaryChildParent = node; - appendAllChildren( - parent, - primaryChildParent, - rootContainerInstance, - childHostContext, - true, - newIsHidden, - ); + appendAllChildren(parent, primaryChildParent, true, newIsHidden); // eslint-disable-next-line no-labels break branches; } @@ -323,8 +289,6 @@ if (supportsMutation) { const appendAllChildrenToContainer = function( containerChildSet: ChildSet, workInProgress: Fiber, - rootContainerInstance: Container, - rootHostContext: HostContext, needsVisibilityToggle: boolean, isHidden: boolean, ) { @@ -355,19 +319,9 @@ if (supportsMutation) { if (needsVisibilityToggle) { const text = node.memoizedProps; if (isHidden) { - instance = createHiddenTextInstance( - text, - rootContainerInstance, - rootHostContext, - workInProgress, - ); + instance = cloneHiddenTextInstance(instance, text, node); } else { - instance = createTextInstance( - text, - rootContainerInstance, - rootHostContext, - workInProgress, - ); + instance = cloneUnhiddenTextInstance(instance, text, node); } node.stateNode = instance; } @@ -386,8 +340,6 @@ if (supportsMutation) { appendAllChildrenToContainer( containerChildSet, primaryChildParent, - rootContainerInstance, - rootHostContext, true, newIsHidden, ); @@ -399,8 +351,6 @@ if (supportsMutation) { appendAllChildrenToContainer( containerChildSet, primaryChildParent, - rootContainerInstance, - rootHostContext, true, newIsHidden, ); @@ -434,11 +384,7 @@ if (supportsMutation) { node = node.sibling; } }; - updateHostContainer = function( - workInProgress: Fiber, - rootContainerInstance: Container, - rootHostContext: HostContext, - ) { + updateHostContainer = function(workInProgress: Fiber) { const portalOrRoot: { containerInfo: Container, pendingChildren: ChildSet, @@ -451,14 +397,7 @@ if (supportsMutation) { const container = portalOrRoot.containerInfo; let newChildSet = createContainerChildSet(container); // If children might have changed, we have to add them all to the set. - appendAllChildrenToContainer( - newChildSet, - workInProgress, - rootContainerInstance, - rootHostContext, - false, - false, - ); + appendAllChildrenToContainer(newChildSet, workInProgress, false, false); portalOrRoot.pendingChildren = newChildSet; // Schedule an update on the container to swap out the container. markUpdate(workInProgress); @@ -471,7 +410,6 @@ if (supportsMutation) { type: Type, newProps: Props, rootContainerInstance: Container, - childHostContext: HostContext, ) { const currentInstance = current.stateNode; const oldProps = current.memoizedProps; @@ -532,14 +470,7 @@ if (supportsMutation) { markUpdate(workInProgress); } else { // If children might have changed, we have to add them all to the set. - appendAllChildren( - newInstance, - workInProgress, - rootContainerInstance, - childHostContext, - false, - false, - ); + appendAllChildren(newInstance, workInProgress, false, false); } }; updateHostText = function( @@ -565,11 +496,7 @@ if (supportsMutation) { }; } else { // No host operations - updateHostContainer = function( - workInProgress: Fiber, - rootContainerInstance: Container, - hostContext: HostContext, - ) { + updateHostContainer = function(workInProgress: Fiber) { // Noop }; updateHostComponent = function( @@ -578,7 +505,6 @@ if (supportsMutation) { type: Type, newProps: Props, rootContainerInstance: Container, - childHostContext: HostContext, ) { // Noop }; @@ -630,19 +556,12 @@ function completeWork( // TODO: Delete this when we delete isMounted and findDOMNode. workInProgress.effectTag &= ~Placement; } - const rootContainerInstance = fiberRoot.containerInfo; - const rootHostContext = getRootHostContext(rootContainerInstance); - updateHostContainer( - workInProgress, - rootContainerInstance, - rootHostContext, - ); + updateHostContainer(workInProgress); break; } case HostComponent: { - const rootContainerInstance = getRootHostContainer(); - const childHostContext = getHostContext(); popHostContext(workInProgress); + const rootContainerInstance = getRootHostContainer(); const type = workInProgress.type; if (current !== null && workInProgress.stateNode != null) { updateHostComponent( @@ -651,7 +570,6 @@ function completeWork( type, newProps, rootContainerInstance, - childHostContext, ); if (current.ref !== workInProgress.ref) { @@ -697,14 +615,7 @@ function completeWork( workInProgress, ); - appendAllChildren( - instance, - workInProgress, - rootContainerInstance, - childHostContext, - false, - false, - ); + appendAllChildren(instance, workInProgress, false, false); // Certain renderers require commit-time effects for initial mount. // (eg DOM renderer supports auto-focus for certain elements). @@ -818,14 +729,8 @@ function completeWork( case Profiler: break; case HostPortal: - const rootContainerInstance = getRootHostContainer(); - const childHostContext = getHostContext(); popHostContainer(workInProgress); - updateHostContainer( - workInProgress, - rootContainerInstance, - childHostContext, - ); + updateHostContainer(workInProgress); break; case ContextProvider: // Pop provider fiber diff --git a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js index 994ca852ad327..4a68193c8534d 100644 --- a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js @@ -92,7 +92,9 @@ export const finalizeContainerChildren = export const replaceContainerChildren = $$$hostConfig.replaceContainerChildren; export const cloneHiddenInstance = $$$hostConfig.cloneHiddenInstance; export const cloneUnhiddenInstance = $$$hostConfig.cloneUnhiddenInstance; -export const createHiddenTextInstance = $$$hostConfig.createHiddenTextInstance; +export const cloneHiddenTextInstance = $$$hostConfig.cloneHiddenTextInstance; +export const cloneUnhiddenTextInstance = + $$$hostConfig.cloneUnhiddenTextInstance; // ------------------- // Hydration diff --git a/packages/shared/HostConfigWithNoPersistence.js b/packages/shared/HostConfigWithNoPersistence.js index 397df41ef0a7b..ffd8342708e34 100644 --- a/packages/shared/HostConfigWithNoPersistence.js +++ b/packages/shared/HostConfigWithNoPersistence.js @@ -30,4 +30,5 @@ export const finalizeContainerChildren = shim; export const replaceContainerChildren = shim; export const cloneHiddenInstance = shim; export const cloneUnhiddenInstance = shim; -export const createHiddenTextInstance = shim; +export const cloneHiddenTextInstance = shim; +export const cloneUnhiddenTextInstance = shim;