RN Inspector guard against clicks outside of RN#9911
Conversation
| measure: callback => | ||
| UIManager.measure(component.getHostNode(), callback), | ||
| props: getHostProps(component), | ||
| source: component._currentElement && component._currentElement._source, |
There was a problem hiding this comment.
Do we need this? It’s only called for items in the hierarchy array which I presume are already not nulls.
There was a problem hiding this comment.
Yes. I only added enough things to prevent the repro steps from crashing.
There was a problem hiding this comment.
This means we’re doing something else that’s bad. How do nulls end up in owner hierarchy?
| UIManager.measure(component.getHostNode(), callback), | ||
| props: getHostProps(component), | ||
| source: component._currentElement && component._currentElement._source, | ||
| source: component && |
There was a problem hiding this comment.
I would probably prefer an early return of “empty hierarchy” rather than these kinds of checks.
There was a problem hiding this comment.
Yup! Testing that now.
There was a problem hiding this comment.
Agreed, that's nicer. Thanks for pointing out the obvious. 😄
| // Handle case where user clicks outside of ReactNative | ||
| if (!closestInstance) { | ||
| return { | ||
| hierarchy: emptyObject, |
e392d6c to
c892d86
Compare
| const instance = lastNotNativeInstance(componentHierarchy); | ||
|
|
||
| // Handle case where user clicks outside of ReactNative | ||
| if (!instance) { |
There was a problem hiding this comment.
Shouldn't we exit earlier, if component is null?
There was a problem hiding this comment.
(which is equivalent to closestInstance in the other version)
There was a problem hiding this comment.
Yeah, that would be better. 😄
c892d86 to
6bf5c50
Compare
Clicking "outside" of a React Native app can cause the latest incarnation of the Inspector to choke. This diff adds guards for this case.