Skip to content

Commit

Permalink
Refactor findNodeHandler (#6736)
Browse files Browse the repository at this point in the history
## Summary

This PR addresses the issue reported at
[https://github.com/software-mansion/react-native-reanimated/issues/6719](https://github.com/software-mansion/react-native-reanimated/issues/6719)
and aims to:

- **Unify the method used to obtain the view tag**, as there are
currently several approaches.
- **Avoid passing a class component to the findNodeHandler**. Instead,
we'll pass a ref to the component, similar to what Expo implemented
here:
[https://github.com/expo/expo/pull/33016](https://github.com/expo/expo/pull/33016).
- **Limit unnecessary invocations of findNodeHandler** to no more than
one call per render.
- **Remove the invocation of findHostInstance from Paper renderer** on
the New Architecture.

**Additional Remarks:**
- When a class component is passed to `createAnimatedComponent`, it will
still fall back to the slow path where we can get a native ref.
- In `NativeEventManager`, we need to call findNodeHandler again after
every render to ensure that the children of `<GestureDetector />`
haven't changed their native tags.
- `LayoutAnimationConfig` still uses findNodeHandler. It requires a
complete refactor of their functionality to eliminate its use, and I
plan to handle this in another PR.
- `findHostInstance_DEPRECATED` always follows the slow path even for
native refs, which is why I've implemented our own version of
`findHostInstance` to optimize the happy path.

Fixes
#6719

Related PRs:
- #6030
- #5960
- #4445
- expo/expo#33016

## Test plan

- [x] check Paper
- [x] check Fabric
- [x] check Web
  • Loading branch information
piaskowyk authored Dec 4, 2024
1 parent f6e54f7 commit 5ffa477
Show file tree
Hide file tree
Showing 9 changed files with 198 additions and 102 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class JSPropsUpdaterPaper implements IJSPropsUpdater {
> &
IAnimatedComponentInternal
) {
const viewTag = animatedComponent._componentViewTag;
const viewTag = animatedComponent.getComponentViewTag();
JSPropsUpdaterPaper._tagToComponentMapping.set(viewTag, animatedComponent);
if (JSPropsUpdaterPaper._tagToComponentMapping.size === 1) {
const listener = (data: ListenerData) => {
Expand All @@ -60,7 +60,7 @@ class JSPropsUpdaterPaper implements IJSPropsUpdater {
> &
IAnimatedComponentInternal
) {
const viewTag = animatedComponent._componentViewTag;
const viewTag = animatedComponent.getComponentViewTag();
JSPropsUpdaterPaper._tagToComponentMapping.delete(viewTag);
if (JSPropsUpdaterPaper._tagToComponentMapping.size === 0) {
this._reanimatedEventEmitter.removeAllListeners(
Expand Down Expand Up @@ -100,7 +100,7 @@ class JSPropsUpdaterFabric implements IJSPropsUpdater {
if (!JSPropsUpdaterFabric.isInitialized) {
return;
}
const viewTag = animatedComponent._componentViewTag;
const viewTag = animatedComponent.getComponentViewTag();
JSPropsUpdaterFabric._tagToComponentMapping.set(viewTag, animatedComponent);
}

Expand All @@ -113,7 +113,7 @@ class JSPropsUpdaterFabric implements IJSPropsUpdater {
if (!JSPropsUpdaterFabric.isInitialized) {
return;
}
const viewTag = animatedComponent._componentViewTag;
const viewTag = animatedComponent.getComponentViewTag();
JSPropsUpdaterFabric._tagToComponentMapping.delete(viewTag);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export class NativeEventsManager implements INativeEventsManager {
public updateEvents(
prevProps: AnimatedComponentProps<InitialComponentProps>
) {
const computedEventTag = this.getEventViewTag();
const computedEventTag = this.getEventViewTag(true);
// If the event view tag changes, we need to completely re-mount all events
if (this.#eventViewTag !== computedEventTag) {
// Remove all bindings from previous props that ran on the old viewTag
Expand Down Expand Up @@ -77,23 +77,54 @@ export class NativeEventsManager implements INativeEventsManager {
});
}

private getEventViewTag() {
private getEventViewTag(componentUpdate: boolean = false) {
// Get the tag for registering events - since the event emitting view can be nested inside the main component
const componentAnimatedRef = this.#managedComponent
._component as AnimatedComponentRef;
let newTag: number;
._componentRef as AnimatedComponentRef & {
// Fabric
__nativeTag?: number;
// Paper
_nativeTag?: number;
};
if (componentAnimatedRef.getScrollableNode) {
/*
In most cases, getScrollableNode() returns a view tag, and findNodeHandle is not required.
However, to cover more exotic list cases, we will continue to use findNodeHandle
for consistency. For numerical values, findNodeHandle should return the value immediately,
as documented here: https://github.com/facebook/react/blob/91061073d57783c061889ac6720ef1ab7f0c2149/packages/react-native-renderer/src/ReactNativePublicCompat.js#L113
*/
const scrollableNode = componentAnimatedRef.getScrollableNode();
newTag = findNodeHandle(scrollableNode) ?? -1;
} else {
newTag =
findNodeHandle(
this.#componentOptions?.setNativeProps
? this.#managedComponent
: componentAnimatedRef
) ?? -1;
if (typeof scrollableNode === 'number') {
return scrollableNode;
}
return findNodeHandle(scrollableNode) ?? -1;
}
if (this.#componentOptions?.setNativeProps) {
// This case ensures backward compatibility with components that
// have their own setNativeProps method passed as an option.
return findNodeHandle(this.#managedComponent) ?? -1;
}
if (!componentUpdate) {
// On the first render of a component, we may already receive a resolved view tag.
return this.#managedComponent.getComponentViewTag();
}
if (componentAnimatedRef.__nativeTag || componentAnimatedRef._nativeTag) {
/*
Fast path for native refs,
_nativeTag is used by Paper components,
__nativeTag is used by Fabric components.
*/
return (
componentAnimatedRef.__nativeTag ??
componentAnimatedRef._nativeTag ??
-1
);
}
return newTag;
/*
When a component is updated, a child could potentially change and have a different
view tag. This can occur with a GestureDetector component.
*/
return findNodeHandle(componentAnimatedRef) ?? -1;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,10 @@ export interface AnimatedComponentRef extends Component {
export interface IAnimatedComponentInternal {
_styles: StyleProps[] | null;
_animatedProps?: Partial<AnimatedComponentProps<AnimatedProps>>;
/**
* Used for Shared Element Transitions, Layout Animations and Animated Styles.
* It is not related to event handling.
*/
_componentViewTag: number;
_isFirstRender: boolean;
jestInlineStyle: NestedArray<StyleProps> | undefined;
jestAnimatedStyle: { value: StyleProps };
_component: AnimatedComponentRef | HTMLElement | null;
_componentRef: AnimatedComponentRef | HTMLElement | null;
_sharedElementTransition: SharedTransition | null;
_jsPropsUpdater: IJSPropsUpdater;
_InlinePropManager: IInlinePropManager;
Expand All @@ -117,6 +112,11 @@ export interface IAnimatedComponentInternal {
_NativeEventsManager?: INativeEventsManager;
_viewInfo?: ViewInfo;
context: React.ContextType<typeof SkipEnteringContext>;
/**
* Used for Shared Element Transitions, Layout Animations and Animated Styles.
* It is not related to event handling.
*/
getComponentViewTag: () => number;
}

export type NestedArray<T> = T | NestedArray<T>[];
Expand Down
Loading

0 comments on commit 5ffa477

Please sign in to comment.