Skip to content

Commit

Permalink
Remove unnecessary (and incorrect) code for compatibility with Paper …
Browse files Browse the repository at this point in the history
…in the Fabric version of GlobalResponderHandler (facebook#26290)

## 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:

```javascript
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.
  • Loading branch information
rubennorte committed Mar 3, 2023
1 parent e64a8f4 commit 06460b6
Showing 1 changed file with 15 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,42 +7,24 @@
* @flow
*/

// Module provided by RN:
import {UIManager} from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface';

const ReactFabricGlobalResponderHandler = {
onChange: function (from: any, to: any, blockNativeResponder: boolean) {
const fromOrTo = from || to;
const fromOrToStateNode = fromOrTo && fromOrTo.stateNode;
const isFabric = !!(
fromOrToStateNode && fromOrToStateNode.canonical._internalInstanceHandle
);

if (isFabric) {
if (from) {
// equivalent to clearJSResponder
nativeFabricUIManager.setIsJSResponder(
from.stateNode.node,
false,
blockNativeResponder || false,
);
}
if (from) {
// equivalent to clearJSResponder
nativeFabricUIManager.setIsJSResponder(
from.stateNode.node,
false,
blockNativeResponder || false,
);
}

if (to) {
// equivalent to setJSResponder
nativeFabricUIManager.setIsJSResponder(
to.stateNode.node,
true,
blockNativeResponder || false,
);
}
} else {
if (to !== null) {
const tag = to.stateNode.canonical._nativeTag;
UIManager.setJSResponder(tag, blockNativeResponder);
} else {
UIManager.clearJSResponder();
}
if (to) {
// equivalent to setJSResponder
nativeFabricUIManager.setIsJSResponder(
to.stateNode.node,
true,
blockNativeResponder || false,
);
}
},
};
Expand Down

0 comments on commit 06460b6

Please sign in to comment.