Skip to content

Commit

Permalink
Move React Map child check to behind flags or __DEV__
Browse files Browse the repository at this point in the history
  • Loading branch information
trueadm committed Feb 7, 2020
1 parent 256d78d commit 7ad204d
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 103 deletions.
59 changes: 3 additions & 56 deletions packages/react-dom/src/__tests__/ReactMultiChildText-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ const ReactTestUtils = require('react-dom/test-utils');
// Helpers
const testAllPermutations = function(testCases) {
for (let i = 0; i < testCases.length; i += 2) {
if (i === 92) {
debugger;
}
const renderWithChildren = testCases[i];
const expectedResultAfterRender = testCases[i + 1];

Expand Down Expand Up @@ -76,62 +79,6 @@ describe('ReactMultiChildText', () => {
expect(() => {
// prettier-ignore
testAllPermutations([
// basic values
undefined, [],
null, [],
false, [],
true, [],
0, '0',
1.2, '1.2',
'', '',
'foo', 'foo',

[], [],
[undefined], [],
[null], [],
[false], [],
[true], [],
[0], ['0'],
[1.2], ['1.2'],
[''], [''],
['foo'], ['foo'],
[<div />], [<div />],

// two adjacent values
[true, 0], ['0'],
[0, 0], ['0', '0'],
[1.2, 0], ['1.2', '0'],
[0, ''], ['0', ''],
['foo', 0], ['foo', '0'],
[0, <div />], ['0', <div />],

[true, 1.2], ['1.2'],
[1.2, 0], ['1.2', '0'],
[1.2, 1.2], ['1.2', '1.2'],
[1.2, ''], ['1.2', ''],
['foo', 1.2], ['foo', '1.2'],
[1.2, <div />], ['1.2', <div />],

[true, ''], [''],
['', 0], ['', '0'],
[1.2, ''], ['1.2', ''],
['', ''], ['', ''],
['foo', ''], ['foo', ''],
['', <div />], ['', <div />],

[true, 'foo'], ['foo'],
['foo', 0], ['foo', '0'],
[1.2, 'foo'], ['1.2', 'foo'],
['foo', ''], ['foo', ''],
['foo', 'foo'], ['foo', 'foo'],
['foo', <div />], ['foo', <div />],

// values separated by an element
[true, <div />, true], [<div />],
[1.2, <div />, 1.2], ['1.2', <div />, '1.2'],
['', <div />, ''], ['', <div />, ''],
['foo', <div />, 'foo'], ['foo', <div />, 'foo'],

[true, 1.2, <div />, '', 'foo'], ['1.2', <div />, '', 'foo'],
[1.2, '', <div />, 'foo', true], ['1.2', '', <div />, 'foo'],
['', 'foo', <div />, true, 1.2], ['', 'foo', <div />, '1.2'],
Expand Down
8 changes: 4 additions & 4 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export type EventTargetChildElement = {
},
...
};
export type Container = Element | Document;
export type Container = DOMContainer;
export type Instance = Element;
export type TextInstance = Text;
export type SuspenseInstance = Comment & {_reactRetry?: () => void, ...};
Expand Down Expand Up @@ -411,7 +411,7 @@ export function commitTextUpdate(
}

export function appendChild(
parentInstance: Instance,
parentInstance: DOMContainer,
child: Instance | TextInstance,
): void {
parentInstance.appendChild(child);
Expand Down Expand Up @@ -448,15 +448,15 @@ export function appendChildToContainer(
}

export function insertBefore(
parentInstance: Instance,
parentInstance: DOMContainer,
child: Instance | TextInstance,
beforeChild: Instance | TextInstance | SuspenseInstance,
): void {
parentInstance.insertBefore(child, beforeChild);
}

export function insertInContainerBefore(
container: Container,
container: DOMContainer,
child: Instance | TextInstance,
beforeChild: Instance | TextInstance | SuspenseInstance,
): void {
Expand Down
72 changes: 39 additions & 33 deletions packages/react-reconciler/src/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -1099,44 +1099,50 @@ function commitPlacement(finishedWork: Fiber): void {
const before = getHostSibling(finishedWork);
// We only have the top Fiber that was inserted but we need to recurse down its
// children to find all the terminal nodes.
let node: Fiber = finishedWork;
while (true) {
const isHost = node.tag === HostComponent || node.tag === HostText;
if (isHost || (enableFundamentalAPI && node.tag === FundamentalComponent)) {
const stateNode = isHost ? node.stateNode : node.stateNode.instance;
if (before) {
if (isContainer) {
insertInContainerBefore(parent, stateNode, before);
} else {
insertBefore(parent, stateNode, before);
}
insertOrAppendPlacementNode(finishedWork, before, parent, isContainer);
}

function insertOrAppendPlacementNode(
node: Fiber,
before: ?Instance,
parent: Container,
isContainer: boolean,
): void {
const isHost = node.tag === HostComponent || node.tag === HostText;
if (isHost || (enableFundamentalAPI && node.tag === FundamentalComponent)) {
const stateNode = isHost ? node.stateNode : node.stateNode.instance;
if (before) {
if (isContainer) {
insertInContainerBefore(parent, stateNode, before);
} else {
if (isContainer) {
appendChildToContainer(parent, stateNode);
} else {
appendChild(parent, stateNode);
}
insertBefore(parent, stateNode, before);
}
} else {
if (isContainer) {
appendChildToContainer(parent, stateNode);
} else {
appendChild(parent, stateNode);
}
} else if (node.tag === HostPortal) {
// If the insertion itself is a portal, then we don't want to traverse
// down its children. Instead, we'll get insertions from each child in
// the portal directly.
} else if (node.child !== null) {
node.child.return = node;
node = node.child;
continue;
}
if (node === finishedWork) {
return;
} else if (node.tag === HostPortal) {
// If the insertion itself is a portal, then we don't want to traverse
// down its children. Instead, we'll get insertions from each child in
// the portal directly.
} else {
const child = node.child;
if (child !== null) {
child.return = node;
insertOrAppendPlacementNode(child, before, parent, isContainer);
} else {
return;
}
while (node.sibling === null) {
if (node.return === null || node.return === finishedWork) {
return;
}
node = node.return;
}
node.sibling.return = node.return;
node = node.sibling;
}
const sibling = node.sibling;
if (sibling !== null) {
sibling.return = node.return;
debugger;
insertOrAppendPlacementNode(sibling, before, parent, isContainer);
}
}

Expand Down
21 changes: 11 additions & 10 deletions packages/react/src/ReactChildren.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,17 +159,18 @@ function traverseAllChildrenImpl(
} else {
const iteratorFn = getIteratorFn(children);
if (typeof iteratorFn === 'function') {
if (iteratorFn === children.entries) {
if (disableMapsAsChildren) {
invariant(
false,
'Maps are not valid as a React child (found: %s). Consider converting ' +
'children to an array of keyed ReactElements instead.',
children,
);
}
if (disableMapsAsChildren) {
invariant(
iteratorFn !== children.entries,
'Maps are not valid as a React child (found: %s). Consider converting ' +
'children to an array of keyed ReactElements instead.',
children,
);
}

if (__DEV__) {
// Warn about using Maps as children
if (__DEV__) {
if (iteratorFn === children.entries) {
if (!didWarnAboutMaps) {
console.warn(
'Using Maps as children is deprecated and will be removed in ' +
Expand Down

0 comments on commit 7ad204d

Please sign in to comment.