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

support Call and Return components in React.Children calls #11422

Merged
merged 6 commits into from Nov 20, 2017
Merged

support Call and Return components in React.Children calls #11422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e8956cb
support Call and Return components in React.Children calls
Nov 1, 2017
c75180e
make tests more verbose
Nov 1, 2017
e76202f
fix ordering of React component types
Nov 1, 2017
3d22e79
cleanup conditional detection of children type
Nov 10, 2017
d20d0c2
directly inline callback invocation
Nov 14, 2017
0a5e5f0
reduce callback invocation code re-use
Nov 20, 2017
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
37 changes: 28 additions & 9 deletions packages/react/src/ReactChildren.js
View file
Open in desktop
Expand Up @@ -25,6 +25,12 @@ var FAUX_ITERATOR_SYMBOL = '@@iterator'; // Before Symbol spec.
var REACT_ELEMENT_TYPE =
(typeof Symbol === 'function' && Symbol.for && Symbol.for('react.element')) ||
0xeac7;
const REACT_CALL_TYPE =
(typeof Symbol === 'function' && Symbol.for && Symbol.for('react.call')) ||
0xeac8;
const REACT_RETURN_TYPE =
(typeof Symbol === 'function' && Symbol.for && Symbol.for('react.return')) ||
0xeac9;
const REACT_PORTAL_TYPE =
(typeof Symbol === 'function' && Symbol.for && Symbol.for('react.portal')) ||
Copy link
Contributor

@nhunzaker nhunzaker Nov 2, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to share the typeof Symbol === 'function' && Symbol.for here? in a supportsSymbolFor variable, or something?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't matter right now. This code executes once. Although it probably makes sense to copy how we do it in ReactChildFiber.

0xeaca;
Expand Down Expand Up @@ -121,15 +127,28 @@ function traverseAllChildrenImpl(
children = null;
}

if (
children === null ||
type === 'string' ||
type === 'number' ||
// The following is inlined from ReactElement. This means we can optimize
// some checks. React Fiber also inlines this logic for similar purposes.
(type === 'object' && children.$$typeof === REACT_ELEMENT_TYPE) ||
(type === 'object' && children.$$typeof === REACT_PORTAL_TYPE)
) {
let invokeCallback = false;

if (children === null) {
invokeCallback = true;
} else {
switch (type) {
case 'string':
case 'number':
invokeCallback = true;
break;
case 'object':
switch (children.$$typeof) {
case REACT_ELEMENT_TYPE:
case REACT_CALL_TYPE:
case REACT_RETURN_TYPE:
case REACT_PORTAL_TYPE:
invokeCallback = true;
}
}
}

if (invokeCallback) {
callback(
traverseContext,
children,
Expand Down
63 changes: 56 additions & 7 deletions packages/react/src/__tests__/ReactChildren-test.js
View file
Open in desktop
Expand Up @@ -58,19 +58,68 @@ describe('ReactChildren', () => {
const portalContainer = document.createElement('div');

const simpleChild = <span key="simple" />;
const portal = ReactDOM.createPortal(simpleChild, portalContainer);
const instance = <div>{portal}</div>;
const reactPortal = ReactDOM.createPortal(simpleChild, portalContainer);

React.Children.forEach(instance.props.children, callback, context);
expect(callback).toHaveBeenCalledWith(portal, 0);
const parentInstance = <div>{reactPortal}</div>;
React.Children.forEach(parentInstance.props.children, callback, context);
expect(callback).toHaveBeenCalledWith(reactPortal, 0);
callback.calls.reset();
const mappedChildren = React.Children.map(
instance.props.children,
parentInstance.props.children,
callback,
context,
);
expect(callback).toHaveBeenCalledWith(reactPortal, 0);
expect(mappedChildren[0]).toEqual(reactPortal);
});

it('should support Call components', () => {
const context = {};
const callback = jasmine.createSpy().and.callFake(function(kid, index) {
expect(this).toBe(context);
return kid;
});
const ReactCallReturn = require('react-call-return');
const reactCall = ReactCallReturn.unstable_createCall(
<span key="simple" />,
() => {},
);

const parentInstance = <div>{reactCall}</div>;
React.Children.forEach(parentInstance.props.children, callback, context);
expect(callback).toHaveBeenCalledWith(reactCall, 0);
callback.calls.reset();
const mappedChildren = React.Children.map(
parentInstance.props.children,
callback,
context,
);
expect(callback).toHaveBeenCalledWith(reactCall, 0);
expect(mappedChildren[0]).toEqual(reactCall);
});

it('should support Return components', () => {
const context = {};
const callback = jasmine.createSpy().and.callFake(function(kid, index) {
expect(this).toBe(context);
return kid;
});
const ReactCallReturn = require('react-call-return');
const reactReturn = ReactCallReturn.unstable_createReturn(
<span key="simple" />,
);

const parentInstance = <div>{reactReturn}</div>;
React.Children.forEach(parentInstance.props.children, callback, context);
expect(callback).toHaveBeenCalledWith(reactReturn, 0);
callback.calls.reset();
const mappedChildren = React.Children.map(
parentInstance.props.children,
callback,
context,
);
expect(callback).toHaveBeenCalledWith(portal, 0);
expect(mappedChildren[0]).toEqual(portal);
expect(callback).toHaveBeenCalledWith(reactReturn, 0);
expect(mappedChildren[0]).toEqual(reactReturn);
});

it('should treat single arrayless child as being in array', () => {
Expand Down