-
Notifications
You must be signed in to change notification settings - Fork 65
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.
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
is "Dynamic" variations of createElement needed? #141
Comments
based on the current implementation of createElement in react, definition of createElement we have result's in more allocation then createElementDynamic, as it needs to copy children from arguments to array. and there is no point in doing this extra work as we already have children array at hand when calling both createElementDynamic and createElement. |
I'm not familiar with React 16 yet but I think it's still necessary for React 15. |
looks like it's same in v15 branch https://github.com/facebook/react/blob/15-stable/src/isomorphic/classic/element/ReactElement.js#L213-L229 |
Good point. The code does look to treat spread arguments as an array. I am
wondering if on React 16, if you pass an array of elements to
createElementDynamic without the elements having a key prop, does React
still issue a warning?
…On Fri, Apr 13, 2018 at 16:40 Irakli Safareli ***@***.***> wrote:
looks like it's same in v15 branch
https://github.com/facebook/react/blob/15-stable/src/isomorphic/classic/element/ReactElement.js#L213-L229
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#141 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAVYy8stSanWzuTCfP923ce11cUEWfh6ks5toQ0kgaJpZM4TTonV>
.
|
AFAIK this is the only difference. When React sees an Array as child, it expects all the elements to have keys. |
If you look at current master of React, in
createElement
there is no difference between spreading children or passing children as array:https://github.com/facebook/react/blob/master/packages/react/src/ReactElement.js#L203-L219
I guess is this spread vs non spread version of createElement not needed any more
it was introduced as of #53
The text was updated successfully, but these errors were encountered: