search

buildo / react-components

Collection of general React components used in buildo projects.
http://react-components.buildo.io/
MIT License
157 stars 21 forks source link

#1237: Update all string refs (closes #1237) #1238

Closed veej closed 6 years ago

veej commented 6 years ago

Closes #1237

Test Plan

tests performed

Tests are still 🍏 Components are still working

giogonzo commented 6 years ago

@veej did you investigate the | null on a callback ref? The react doc say:

React will call the ref callback with the DOM element when the component mounts, and call it with null when it unmounts. ref callbacks are invoked before componentDidMount or componentDidUpdate lifecycle hooks.

and:

If the ref callback is defined as an inline function, it will get called twice during updates, first with null and then again with the DOM element. This is because a new instance of the function is created with each render, so React needs to clear the old ref and set up the new one. You can avoid this by defining the ref callback as a bound method on the class, but note that it shouldn’t matter in most cases.

veej commented 6 years ago

@giogonzo yep, I found the same thing, and I think it should not be a problem to consider the ref as non-null where we use it. What's your opinion? A safer alternative could be to move the ! wherever we use the ref (only if we're sure the component is mounted) of check for null every time we read it.

nemobot commented 6 years ago