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.

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

getFirst usage currently can result in three instances #880

Open
jhsu opened this issue Jan 17, 2018 · 1 comment
Open

getFirst usage currently can result in three instances #880

jhsu opened this issue Jan 17, 2018 · 1 comment
Labels

Comments

@jhsu
Copy link
Collaborator

jhsu commented Jan 17, 2018

As stated in #879, getFirst can result in two instances. Then later when the results of getFirst are used, we simply grab the props off the created element and create a new element such as:

const dateSelectChild = getFirst(this.props, DatePicker.DateSelect, <DatePicker.DateSelect />);
...
<DateSelect
  {...dateSelectChild.props} />

which results in 3 instances. Should we update getFirst to accept additional props or return a constructor?

@ogupte
Copy link
Contributor

ogupte commented Jan 18, 2018

At most there are 2 elements of DatePicker.DateSelect created, and 1 element of DateSelect is created. These are distinct components as DatePicker.DateSelect has render: () => null since it's only used to pass props. Meanwhile the DateSelect element created in DatePicker's render function is the fully rendered DateSelect with render logic. The latter, expensive component is always rendered regardless of getFirst's code path, so performance gains from minimizing element creation of DatePicker.DateSelect would not be as great as you might expect.

Regardless, i like the idea of passing props to getFirst that minimizes element creation.

@jondlm jondlm added the quality label Oct 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants