Refactor findHostInstance and findNodeHandle #12575
Conversation
sebmarkbage
force-pushed
the
findnodehandle
branch
2 times, most recently
from
d2ac7f6
to
f97ce2e
Compare
| @@ -402,7 +403,19 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>( | |||
| ); | |||
| } | |||
|
|
|||
| function findHostInstance(fiber: Fiber): PI | null { | |||
| function findHostInstance(component: Object): PI | null { | |||
There was a problem hiding this comment.
I don't think this is safe because findHostInstance is being passed to DevTools below:
react/packages/react-reconciler/src/ReactFiberReconciler.js
Lines 508 to 512 in 76b4ba0
DevTools assumes it can pass a Fiber there.
There was a problem hiding this comment.
Seems like we should be injecting into devtools from inside the reconciler. Or at least expose something opaque to inject into devtools.
There was a problem hiding this comment.
Aren't we already? This code is in the reconciler. I'm just pointing out that I think you changed the signature of the function you're passing.
There was a problem hiding this comment.
ah. I see. Also, spread in production. Sad panda. :(
There was a problem hiding this comment.
We really should get rid of that. We don't want it in the reconciler so the devtools dep should go away. If it really need it, it can traverse the tree itself, since it is already Fiber aware. (If it even needs it since it has its own shadow tree anyway.)
There was a problem hiding this comment.
Patched it for now.
sebmarkbage
force-pushed
the
findnodehandle
branch
from
f97ce2e
to
769212f
Compare
|
Yea, I tested it before and after the fix. Seems to have fixed it. |
sebmarkbage
force-pushed
the
findnodehandle
branch
2 times, most recently
from
b3c51d1
to
4ab3cc3
Compare
This is just like ReactDOM does it. This also lets us get rid of injection for findNodeHandle. Instead I move NativeMethodsMixin and ReactNativeComponent to use instantiation.
The reconciler shouldn't expose the Fiber data structure. We should pass the component instance to the reconciler, since the reconciler is the thing that is supposed to be instancemap aware.
sebmarkbage
force-pushed
the
findnodehandle
branch
from
4ab3cc3
to
cf454c5
Compare
* Move findNodeHandle into the renderers and use instantiation This is just like ReactDOM does it. This also lets us get rid of injection for findNodeHandle. Instead I move NativeMethodsMixin and ReactNativeComponent to use instantiation. * Refactor findHostInstance The reconciler shouldn't expose the Fiber data structure. We should pass the component instance to the reconciler, since the reconciler is the thing that is supposed to be instancemap aware. * Fix devtools injection
The reconciler shouldn't expose the Fiber data structure. We should pass the component instance to the reconciler, since the reconciler is the thing that is supposed to be instancemap aware.
Move findNodeHandle into the renderers and use instantiation
This is just like ReactDOM does it. This also lets us get rid of injection for findNodeHandle. Instead I move NativeMethodsMixin and ReactNativeComponent to use instantiation.