From e8956cbc57ce63a650be4354bfcdc67cfbee5143 Mon Sep 17 00:00:00 2001 From: Matteo Vesprini-Heidrich Date: Tue, 31 Oct 2017 22:23:43 -0400 Subject: [PATCH 1/6] support Call and Return components in React.Children calls --- packages/react/src/ReactChildren.js | 10 +++- .../react/src/__tests__/ReactChildren-test.js | 55 +++++++++++++++---- 2 files changed, 53 insertions(+), 12 deletions(-) diff --git a/packages/react/src/ReactChildren.js b/packages/react/src/ReactChildren.js index 346668b59bc6c..ba53d413dec7c 100644 --- a/packages/react/src/ReactChildren.js +++ b/packages/react/src/ReactChildren.js @@ -28,6 +28,12 @@ var REACT_ELEMENT_TYPE = const REACT_PORTAL_TYPE = (typeof Symbol === 'function' && Symbol.for && Symbol.for('react.portal')) || 0xeaca; +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; var SEPARATOR = '.'; var SUBSEPARATOR = ':'; @@ -128,7 +134,9 @@ function traverseAllChildrenImpl( // 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) + (type === 'object' && children.$$typeof === REACT_PORTAL_TYPE) || + (type === 'object' && children.$$typeof === REACT_CALL_TYPE) || + (type === 'object' && children.$$typeof === REACT_RETURN_TYPE) ) { callback( traverseContext, diff --git a/packages/react/src/__tests__/ReactChildren-test.js b/packages/react/src/__tests__/ReactChildren-test.js index d06e965ea7b9c..6cb6ea7f4d151 100644 --- a/packages/react/src/__tests__/ReactChildren-test.js +++ b/packages/react/src/__tests__/ReactChildren-test.js @@ -23,6 +23,20 @@ describe('ReactChildren', () => { ReactTestUtils = require('react-dom/test-utils'); }); + const checkReactChildrenFunctionality = (element, callback, context) => { + const parentInstance =
{element}
; + React.Children.forEach(parentInstance.props.children, callback, context); + expect(callback).toHaveBeenCalledWith(element, 0); + callback.calls.reset(); + const mappedChildren = React.Children.map( + parentInstance.props.children, + callback, + context, + ); + expect(callback).toHaveBeenCalledWith(element, 0); + expect(mappedChildren[0]).toEqual(element); + }; + it('should support identity for simple', () => { var context = {}; var callback = jasmine.createSpy().and.callFake(function(kid, index) { @@ -58,19 +72,38 @@ describe('ReactChildren', () => { const portalContainer = document.createElement('div'); const simpleChild = ; - const portal = ReactDOM.createPortal(simpleChild, portalContainer); - const instance =
{portal}
; + const reactPortal = ReactDOM.createPortal(simpleChild, portalContainer); - React.Children.forEach(instance.props.children, callback, context); - expect(callback).toHaveBeenCalledWith(portal, 0); - callback.calls.reset(); - const mappedChildren = React.Children.map( - instance.props.children, - callback, - context, + checkReactChildrenFunctionality(reactPortal, callback, context); + }); + + 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 reactReturn = ReactCallReturn.unstable_createCall( + , + () => {}, + ); + + checkReactChildrenFunctionality(reactReturn, callback, context); + }); + + 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( + , ); - expect(callback).toHaveBeenCalledWith(portal, 0); - expect(mappedChildren[0]).toEqual(portal); + + checkReactChildrenFunctionality(reactReturn, callback, context); }); it('should treat single arrayless child as being in array', () => { From c75180e0675b824e53ce16e557e3435403cf4218 Mon Sep 17 00:00:00 2001 From: Matteo Vesprini-Heidrich Date: Wed, 1 Nov 2017 14:30:38 -0400 Subject: [PATCH 2/6] make tests more verbose --- .../react/src/__tests__/ReactChildren-test.js | 52 ++++++++++++------- 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/packages/react/src/__tests__/ReactChildren-test.js b/packages/react/src/__tests__/ReactChildren-test.js index 6cb6ea7f4d151..c41a2b8cb699d 100644 --- a/packages/react/src/__tests__/ReactChildren-test.js +++ b/packages/react/src/__tests__/ReactChildren-test.js @@ -23,20 +23,6 @@ describe('ReactChildren', () => { ReactTestUtils = require('react-dom/test-utils'); }); - const checkReactChildrenFunctionality = (element, callback, context) => { - const parentInstance =
{element}
; - React.Children.forEach(parentInstance.props.children, callback, context); - expect(callback).toHaveBeenCalledWith(element, 0); - callback.calls.reset(); - const mappedChildren = React.Children.map( - parentInstance.props.children, - callback, - context, - ); - expect(callback).toHaveBeenCalledWith(element, 0); - expect(mappedChildren[0]).toEqual(element); - }; - it('should support identity for simple', () => { var context = {}; var callback = jasmine.createSpy().and.callFake(function(kid, index) { @@ -74,7 +60,17 @@ describe('ReactChildren', () => { const simpleChild = ; const reactPortal = ReactDOM.createPortal(simpleChild, portalContainer); - checkReactChildrenFunctionality(reactPortal, callback, context); + const parentInstance =
{reactPortal}
; + React.Children.forEach(parentInstance.props.children, callback, context); + expect(callback).toHaveBeenCalledWith(reactPortal, 0); + callback.calls.reset(); + const mappedChildren = React.Children.map( + parentInstance.props.children, + callback, + context, + ); + expect(callback).toHaveBeenCalledWith(reactPortal, 0); + expect(mappedChildren[0]).toEqual(reactPortal); }); it('should support Call components', () => { @@ -84,12 +80,22 @@ describe('ReactChildren', () => { return kid; }); const ReactCallReturn = require('react-call-return'); - const reactReturn = ReactCallReturn.unstable_createCall( + const reactCall = ReactCallReturn.unstable_createCall( , () => {}, ); - checkReactChildrenFunctionality(reactReturn, callback, context); + const parentInstance =
{reactCall}
; + 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', () => { @@ -103,7 +109,17 @@ describe('ReactChildren', () => { , ); - checkReactChildrenFunctionality(reactReturn, callback, context); + const parentInstance =
{reactReturn}
; + 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(reactReturn, 0); + expect(mappedChildren[0]).toEqual(reactReturn); }); it('should treat single arrayless child as being in array', () => { From e76202f888ad083462d17a7961cd9eda9bc272be Mon Sep 17 00:00:00 2001 From: Matteo Vesprini-Heidrich Date: Wed, 1 Nov 2017 14:30:48 -0400 Subject: [PATCH 3/6] fix ordering of React component types --- packages/react/src/ReactChildren.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/react/src/ReactChildren.js b/packages/react/src/ReactChildren.js index ba53d413dec7c..fa8499830c1ed 100644 --- a/packages/react/src/ReactChildren.js +++ b/packages/react/src/ReactChildren.js @@ -25,15 +25,15 @@ var FAUX_ITERATOR_SYMBOL = '@@iterator'; // Before Symbol spec. var REACT_ELEMENT_TYPE = (typeof Symbol === 'function' && Symbol.for && Symbol.for('react.element')) || 0xeac7; -const REACT_PORTAL_TYPE = - (typeof Symbol === 'function' && Symbol.for && Symbol.for('react.portal')) || - 0xeaca; 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')) || + 0xeaca; var SEPARATOR = '.'; var SUBSEPARATOR = ':'; @@ -134,9 +134,9 @@ function traverseAllChildrenImpl( // 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) || (type === 'object' && children.$$typeof === REACT_CALL_TYPE) || - (type === 'object' && children.$$typeof === REACT_RETURN_TYPE) + (type === 'object' && children.$$typeof === REACT_RETURN_TYPE) || + (type === 'object' && children.$$typeof === REACT_PORTAL_TYPE) ) { callback( traverseContext, From 3d22e79df05b776b38596406eb7a4edac5919d2d Mon Sep 17 00:00:00 2001 From: Matteo Vesprini-Heidrich Date: Fri, 10 Nov 2017 14:52:15 -0500 Subject: [PATCH 4/6] cleanup conditional detection of children type --- packages/react/src/ReactChildren.js | 42 ++++++++++++++++++----------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/packages/react/src/ReactChildren.js b/packages/react/src/ReactChildren.js index fa8499830c1ed..ae3eb851f5c86 100644 --- a/packages/react/src/ReactChildren.js +++ b/packages/react/src/ReactChildren.js @@ -122,22 +122,7 @@ function traverseAllChildrenImpl( ) { var type = typeof children; - if (type === 'undefined' || type === 'boolean') { - // All of the above are perceived as null. - 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_CALL_TYPE) || - (type === 'object' && children.$$typeof === REACT_RETURN_TYPE) || - (type === 'object' && children.$$typeof === REACT_PORTAL_TYPE) - ) { + const invokeCallback = () => { callback( traverseContext, children, @@ -145,9 +130,34 @@ function traverseAllChildrenImpl( // so that it's consistent if the number of children grows. nameSoFar === '' ? SEPARATOR + getComponentKey(children, 0) : nameSoFar, ); + }; + + if (type === 'undefined' || type === 'boolean') { + // All of the above are perceived as null. + children = null; + } + + if (children === null) { + invokeCallback(); return 1; } + switch (type) { + case 'string': + case 'number': + invokeCallback(); + return 1; + case 'object': + switch (children.$$typeof) { + case REACT_ELEMENT_TYPE: + case REACT_CALL_TYPE: + case REACT_RETURN_TYPE: + case REACT_PORTAL_TYPE: + invokeCallback(); + return 1; + } + } + var child; var nextName; var subtreeCount = 0; // Count of children found in the current subtree. From d20d0c2be36893c86acbd102221b26de29f79dc6 Mon Sep 17 00:00:00 2001 From: Matteo Vesprini-Heidrich Date: Tue, 14 Nov 2017 11:14:17 -0500 Subject: [PATCH 5/6] directly inline callback invocation --- packages/react/src/ReactChildren.js | 34 +++++++++++++++++++---------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/packages/react/src/ReactChildren.js b/packages/react/src/ReactChildren.js index ae3eb851f5c86..23dfbf72fdb1c 100644 --- a/packages/react/src/ReactChildren.js +++ b/packages/react/src/ReactChildren.js @@ -122,7 +122,12 @@ function traverseAllChildrenImpl( ) { var type = typeof children; - const invokeCallback = () => { + if (type === 'undefined' || type === 'boolean') { + // All of the above are perceived as null. + children = null; + } + + if (children === null) { callback( traverseContext, children, @@ -130,22 +135,19 @@ function traverseAllChildrenImpl( // so that it's consistent if the number of children grows. nameSoFar === '' ? SEPARATOR + getComponentKey(children, 0) : nameSoFar, ); - }; - - if (type === 'undefined' || type === 'boolean') { - // All of the above are perceived as null. - children = null; - } - - if (children === null) { - invokeCallback(); return 1; } switch (type) { case 'string': case 'number': - invokeCallback(); + callback( + traverseContext, + children, + // If it's the only child, treat the name as if it was wrapped in an array + // so that it's consistent if the number of children grows. + nameSoFar === '' ? SEPARATOR + getComponentKey(children, 0) : nameSoFar, + ); return 1; case 'object': switch (children.$$typeof) { @@ -153,7 +155,15 @@ function traverseAllChildrenImpl( case REACT_CALL_TYPE: case REACT_RETURN_TYPE: case REACT_PORTAL_TYPE: - invokeCallback(); + callback( + traverseContext, + children, + // If it's the only child, treat the name as if it was wrapped in an array + // so that it's consistent if the number of children grows. + nameSoFar === '' + ? SEPARATOR + getComponentKey(children, 0) + : nameSoFar, + ); return 1; } } From 0a5e5f006cac876ccac634d11e9f006296e06790 Mon Sep 17 00:00:00 2001 From: Matteo Vesprini-Heidrich Date: Mon, 20 Nov 2017 16:20:39 -0500 Subject: [PATCH 6/6] reduce callback invocation code re-use --- packages/react/src/ReactChildren.js | 51 ++++++++++++----------------- 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/packages/react/src/ReactChildren.js b/packages/react/src/ReactChildren.js index 23dfbf72fdb1c..975dedd538f62 100644 --- a/packages/react/src/ReactChildren.js +++ b/packages/react/src/ReactChildren.js @@ -127,7 +127,28 @@ function traverseAllChildrenImpl( children = null; } + 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, @@ -138,36 +159,6 @@ function traverseAllChildrenImpl( return 1; } - switch (type) { - case 'string': - case 'number': - callback( - traverseContext, - children, - // If it's the only child, treat the name as if it was wrapped in an array - // so that it's consistent if the number of children grows. - nameSoFar === '' ? SEPARATOR + getComponentKey(children, 0) : nameSoFar, - ); - return 1; - case 'object': - switch (children.$$typeof) { - case REACT_ELEMENT_TYPE: - case REACT_CALL_TYPE: - case REACT_RETURN_TYPE: - case REACT_PORTAL_TYPE: - callback( - traverseContext, - children, - // If it's the only child, treat the name as if it was wrapped in an array - // so that it's consistent if the number of children grows. - nameSoFar === '' - ? SEPARATOR + getComponentKey(children, 0) - : nameSoFar, - ); - return 1; - } - } - var child; var nextName; var subtreeCount = 0; // Count of children found in the current subtree.