Remove unnecessary (and incorrect) code for compatibility with Paper in the Fabric version of GlobalResponderHandler

[rubennorte]

Summary

I'm working on a refactor of the definition of Instance in Fabric and I came across this code that seemed to be for compatibility with Paper, but that it would actually throw an error in that case.

In Paper, stateNode is an instance of ReactNativeFiberHostComponent, which doesn't have a canonical field. We try to access nested properties in that field in a couple of places here, which would throw a type error (cannot read property _nativeTag of undefined) if we actually happened to pass a reference to a Paper state node.

In this line:

const isFabric = !!(
      fromOrToStateNode && fromOrToStateNode.canonical._internalInstanceHandle
    );

If it wasn't Fabric, fromOrToStateNode.canonical would be undefined, and we don't check for that before accessing fromOrToStateNode.canonical._internalInstanceHandle. This means that we actually never use this logic in Paper or we would've seen the error.

How did you test this change?

Existing tests.

[react-sizebot]

Comparing: b72ed69...4bb596b

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	=	155.25 kB	155.25 kB	=	48.98 kB	48.98 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	157.24 kB	157.24 kB	=	49.64 kB	49.64 kB
facebook-www/ReactDOM-prod.classic.js	=	532.50 kB	532.50 kB	=	94.85 kB	94.85 kB
facebook-www/ReactDOM-prod.modern.js	=	516.42 kB	516.42 kB	=	92.45 kB	92.45 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 4bb596b

[fkgozali]

Nice catch, as long as this fabric handler doesn't get used in any "mixed mode" with legacy renderer, seems reasonable to me.

[mdvacca]

as long as this fabric handler doesn't get used in any "mixed mode"

This is a good point, have we validate this? could we run in a mix mode as part of the rollout of the new architecture in OSS?

CC @cortinico

[rubennorte]

as long as this fabric handler doesn't get used in any "mixed mode"

This is a good point, have we validate this? could we run in a mix mode as part of the rollout of the new architecture in OSS?

CC @cortinico

We already do this in some parts. For example, in the methods exported by ReactNativeRenderer and ReactFabric we support both renderers, in case someone imported them directly. This is properly covered by unit tests. In this specific case, it seems it's not used by anyone externally or it would've failed (and we had this mixed mode for a while at Meta).