lucid icon indicating copy to clipboard operation
lucid copied to clipboard

getFirst usage currently can result in three instances

Open jhsu opened this issue 7 years ago • 1 comments

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?

jhsu avatar Jan 17 '18 23:01 jhsu

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.

ogupte avatar Jan 18 '18 06:01 ogupte