Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Fabric] Use container node to toggle the visibility of Offscreen and Suspense trees #21960

Merged
merged 3 commits into from Jul 26, 2021
Merged

[Fabric] Use container node to toggle the visibility of Offscreen and Suspense trees #21960

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
108faed
Fix type of Offscreen props argument
acdlite Jul 22, 2021
1d8eac9
Extract createOffscreenFromFiber logic
acdlite Jul 22, 2021
4a93531
[Fabric] Use container node to hide/show tree
acdlite Jul 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
50 changes: 28 additions & 22 deletions packages/react-native-renderer/src/ReactFabricHostConfig.js
View file
Open in desktop
Expand Up @@ -7,6 +7,7 @@
* @flow
*/

import type {ReactNodeList, OffscreenMode} from 'shared/ReactTypes';
import type {ElementRef} from 'react';
import type {
HostComponent,
Expand Down Expand Up @@ -301,6 +302,9 @@ export function getChildHostContext(
type === 'RCTText' ||
type === 'RCTVirtualText';

// TODO: If this is an offscreen host container, we should reuse the
// parent context.

if (prevIsInAParentText !== isInAParentText) {
return {isInAParentText};
} else {
Expand Down Expand Up @@ -413,30 +417,32 @@ export function cloneInstance(
};
}

export function cloneHiddenInstance(
instance: Instance,
type: string,
props: Props,
internalInstanceHandle: Object,
): Instance {
const viewConfig = instance.canonical.viewConfig;
const node = instance.node;
const updatePayload = create(
{style: {display: 'none'}},
viewConfig.validAttributes,
);
return {
node: cloneNodeWithNewProps(node, updatePayload),
canonical: instance.canonical,
};
// TODO: These two methods should be replaced with `createOffscreenInstance` and
// `cloneOffscreenInstance`. I did it this way for now because the offscreen
// instance is stored on an extra HostComponent fiber instead of the
// OffscreenComponent fiber, and I didn't want to add an extra check to the
// generic HostComponent path. Instead we should use the OffscreenComponent
// fiber, but currently Fabric expects a 1:1 correspondence between Fabric
// instances and host fibers, so I'm leaving this optimization for later once
// we can confirm this won't break any downstream expectations.
export function getOffscreenContainerType(): string {
return 'RCTView';
}

export function cloneHiddenTextInstance(
instance: Instance,
text: string,
internalInstanceHandle: Object,
): TextInstance {
throw new Error('Not yet implemented.');
export function getOffscreenContainerProps(
mode: OffscreenMode,
children: ReactNodeList,
): Props {
if (mode === 'hidden') {
return {
children,
style: {display: 'none'},
};
} else {
return {
children,
};
}
}

export function createContainerChildSet(container: Container): ChildSet {
Expand Down
200 changes: 159 additions & 41 deletions packages/react-noop-renderer/src/createReactNoop.js
View file
Open in desktop
Expand Up @@ -16,7 +16,7 @@

import type {Fiber} from 'react-reconciler/src/ReactInternalTypes';
import type {UpdateQueue} from 'react-reconciler/src/ReactUpdateQueue';
import type {ReactNodeList} from 'shared/ReactTypes';
import type {ReactNodeList, OffscreenMode} from 'shared/ReactTypes';
import type {RootTag} from 'react-reconciler/src/ReactRootTags';

import * as Scheduler from 'scheduler/unstable_mock';
Expand Down Expand Up @@ -258,6 +258,9 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
type: string,
rootcontainerInstance: Container,
) {
if (type === 'offscreen') {
return parentHostContext;
}
if (type === 'uppercase') {
return UPPERCASE_CONTEXT;
}
Expand Down Expand Up @@ -539,47 +542,18 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
container.children = newChildren;
},

cloneHiddenInstance(
instance: Instance,
type: string,
props: Props,
internalInstanceHandle: Object,
): Instance {
const clone = cloneInstance(
instance,
null,
type,
props,
props,
internalInstanceHandle,
true,
null,
);
clone.hidden = true;
return clone;
getOffscreenContainerType(): string {
return 'offscreen';
},

cloneHiddenTextInstance(
instance: TextInstance,
text: string,
internalInstanceHandle: Object,
): TextInstance {
const clone = {
text: instance.text,
id: instanceCounter++,
hidden: true,
context: instance.context,
getOffscreenContainerProps(
mode: OffscreenMode,
children: ReactNodeList,
): Props {
return {
hidden: mode === 'hidden',
children,
};
// Hide from unit tests
Object.defineProperty(clone, 'id', {
value: clone.id,
enumerable: false,
});
Object.defineProperty(clone, 'context', {
value: clone.context,
enumerable: false,
});
return clone;
},
};

Expand Down Expand Up @@ -646,20 +620,164 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {

function getChildren(root) {
if (root) {
return root.children;
return useMutation
? root.children
: removeOffscreenContainersFromChildren(root.children, false);
} else {
return null;
}
}

function getPendingChildren(root) {
if (root) {
return root.pendingChildren;
return useMutation
? root.children
: removeOffscreenContainersFromChildren(root.pendingChildren, false);
} else {
return null;
}
}

function removeOffscreenContainersFromChildren(children, hideNearestNode) {
// Mutation mode and persistent mode have different outputs for Offscreen
// and Suspense trees. Persistent mode adds an additional host node wrapper,
// whereas mutation mode does not.
//
// This function removes the offscreen host wrappers so that the output is
// consistent. If the offscreen node is hidden, it transfers the hiddenness
// to the child nodes, to mimic how it works in mutation mode. That way our
// tests don't have to fork tree assertions.
//
// So, it takes a tree that looks like this:
//
// <offscreen hidden={true}>
// <span>A</span>
// <span>B</span>
// </offscren>
//
// And turns it into this:
//
// <span hidden={true}>A</span>
// <span hidden={true}>B</span>
//
// We don't mutate the original tree, but instead return a copy.
//
// This function is only used by our test assertions, via the `getChildren`
// and `getChildrenAsJSX` methods.
let didClone = false;
const newChildren = [];
for (let i = 0; i < children.length; i++) {
const child = children[i];
const innerChildren = child.children;
if (innerChildren !== undefined) {
// This is a host instance instance
const instance: Instance = (child: any);
if (instance.type === 'offscreen') {
// This is an offscreen wrapper instance. Remove it from the tree
// and recursively return its children, as if it were a fragment.
didClone = true;
if (instance.text !== null) {
// If this offscreen tree contains only text, we replace it with
// a text child. Related to `shouldReplaceTextContent` feature.
const offscreenTextInstance: TextInstance = {
text: instance.text,
id: instanceCounter++,
hidden: hideNearestNode || instance.hidden,
context: instance.context,
};
// Hide from unit tests
Object.defineProperty(offscreenTextInstance, 'id', {
value: offscreenTextInstance.id,
enumerable: false,
});
Object.defineProperty(offscreenTextInstance, 'context', {
value: offscreenTextInstance.context,
enumerable: false,
});
newChildren.push(offscreenTextInstance);
} else {
// Skip the offscreen node and replace it with its children
const offscreenChildren = removeOffscreenContainersFromChildren(
innerChildren,
hideNearestNode || instance.hidden,
);
newChildren.push.apply(newChildren, offscreenChildren);
}
} else {
// This is a regular (non-offscreen) instance. If the nearest
// offscreen boundary is hidden, hide this node.
const hidden = hideNearestNode ? true : instance.hidden;
const clonedChildren = removeOffscreenContainersFromChildren(
instance.children,
// We never need to hide the children of this node, since if we're
// inside a hidden tree, then the hidden style will be applied to
// this node.
false,
);
if (
clonedChildren === instance.children &&
hidden === instance.hidden
) {
// No changes. Reuse the original instance without cloning.
newChildren.push(instance);
} else {
didClone = true;
const clone: Instance = {
id: instance.id,
type: instance.type,
children: clonedChildren,
text: instance.text,
prop: instance.prop,
hidden: hideNearestNode ? true : instance.hidden,
context: instance.context,
};
Object.defineProperty(clone, 'id', {
value: clone.id,
enumerable: false,
});
Object.defineProperty(clone, 'text', {
value: clone.text,
enumerable: false,
});
Object.defineProperty(clone, 'context', {
value: clone.context,
enumerable: false,
});
newChildren.push(clone);
}
}
} else {
// This is a text instance
const textInstance: TextInstance = (child: any);
if (hideNearestNode) {
didClone = true;
const clone = {
text: textInstance.text,
id: textInstance.id,
hidden: textInstance.hidden || hideNearestNode,
context: textInstance.context,
};
Object.defineProperty(clone, 'id', {
value: clone.id,
enumerable: false,
});
Object.defineProperty(clone, 'context', {
value: clone.context,
enumerable: false,
});

newChildren.push(clone);
} else {
newChildren.push(textInstance);
}
}
}
// There are some tests that assume reference equality, so preserve it
// when possible. Alternatively, we could update the tests to compare the
// ids instead.
return didClone ? newChildren : children;
}

function getChildrenAsJSX(root) {
const children = childToJSX(getChildren(root), null);
if (children === null) {
Expand Down
25 changes: 24 additions & 1 deletion packages/react-reconciler/src/ReactFiber.new.js
View file
Open in desktop
Expand Up @@ -14,7 +14,7 @@ import type {RootTag} from './ReactRootTags';
import type {WorkTag} from './ReactWorkTags';
import type {TypeOfMode} from './ReactTypeOfMode';
import type {Lanes} from './ReactFiberLane.new';
import type {SuspenseInstance} from './ReactFiberHostConfig';
import type {SuspenseInstance, Props} from './ReactFiberHostConfig';
import type {OffscreenProps} from './ReactFiberOffscreenComponent';

import invariant from 'shared/invariant';
Expand All @@ -27,6 +27,10 @@ import {
enableSyncDefaultUpdates,
allowConcurrentByDefault,
} from 'shared/ReactFeatureFlags';
import {
supportsPersistence,
getOffscreenContainerType,
} from './ReactFiberHostConfig';
import {NoFlags, Placement, StaticMask} from './ReactFiberFlags';
import {ConcurrentRoot} from './ReactRootTags';
import {
Expand Down Expand Up @@ -585,6 +589,25 @@ export function createFiberFromTypeAndProps(
return fiber;
}

export function createOffscreenHostContainerFiber(
props: Props,
fiberMode: TypeOfMode,
lanes: Lanes,
key: null | string,
): Fiber {
if (supportsPersistence) {
const type = getOffscreenContainerType();
const fiber = createFiber(HostComponent, props, key, fiberMode);
fiber.elementType = type;
fiber.type = type;
fiber.lanes = lanes;
return fiber;
} else {
// Only implemented in persistent mode
invariant(false, 'Not implemented.');
}
}

export function createFiberFromElement(
element: ReactElement,
mode: TypeOfMode,
Expand Down