support Call and Return components in React.Children calls #11422
Conversation
packages/react/src/ReactChildren.js
Outdated
| @@ -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 = | |||
There was a problem hiding this comment.
Minor nit: let's reorder them before the portal (the numbers go in sequential order).
| @@ -23,6 +23,20 @@ describe('ReactChildren', () => { | |||
| ReactTestUtils = require('react-dom/test-utils'); | |||
| }); | |||
|
|
|||
| const checkReactChildrenFunctionality = (element, callback, context) => { | |||
| const parentInstance = <div>{element}</div>; | |||
There was a problem hiding this comment.
Let's just copy and paste this part between tests. It's fine for tests to be verbose. That way they're easier to debug.
|
Ready for re-review! |
packages/react/src/ReactChildren.js
Outdated
| @@ -128,6 +134,8 @@ 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) || | |||
There was a problem hiding this comment.
Can we return this into a switch somehow?
| 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')) || |
There was a problem hiding this comment.
Does it make sense to share the typeof Symbol === 'function' && Symbol.for here? in a supportsSymbolFor variable, or something?
There was a problem hiding this comment.
Doesn't matter right now. This code executes once. Although it probably makes sense to copy how we do it in ReactChildFiber.
|
Friendly ping :-) |
|
Will have time tonight to re-factor! |
MatteoVH
force-pushed
the
react-children-call-return-support
branch
from
ba323c9
to
7eb6bba
Compare
|
Sorry for the late response, I tried to move some of the if logic into a switch, and it just didn't seem to cleanly fit into that construct. I left one attempt, but I think it's more legible as it was before @gaearon. |
MatteoVH
force-pushed
the
react-children-call-return-support
branch
from
7eb6bba
to
3d22e79
Compare
packages/react/src/ReactChildren.js
Outdated
| (type === 'object' && children.$$typeof === REACT_ELEMENT_TYPE) || | ||
| (type === 'object' && children.$$typeof === REACT_PORTAL_TYPE) | ||
| ) { | ||
| const invokeCallback = () => { |
There was a problem hiding this comment.
This is a very hot path. We don't want to create closures here. It's fine to duplicate code instead if necessary.
|
Instead of duplicating, can we just set a boolean flag? It starts out being |
|
Nice. Thanks. |
…11422) * support Call and Return components in React.Children calls * make tests more verbose * fix ordering of React component types * cleanup conditional detection of children type * directly inline callback invocation * reduce callback invocation code re-use
…11422) * support Call and Return components in React.Children calls * make tests more verbose * fix ordering of React component types * cleanup conditional detection of children type * directly inline callback invocation * reduce callback invocation code re-use
Solves #11416
@gaearon, I tried to read up a bit on Call and Return, but I'm (obviously seeing as it's an experimental API) inexperienced with them. Let me know if I'm testing their usage satisfactorily!