Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor findNodeHandler #6736

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,52 @@ 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 & {
__nativeTag?: number;
_nativeTag?: number;
piaskowyk marked this conversation as resolved.
Show resolved Hide resolved
};
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
*/
piaskowyk marked this conversation as resolved.
Show resolved Hide resolved
const scrollableNode = componentAnimatedRef.getScrollableNode();
newTag = findNodeHandle(scrollableNode) ?? -1;
} else {
newTag =
findNodeHandle(
this.#componentOptions?.setNativeProps
? this.#managedComponent
: componentAnimatedRef
) ?? -1;
if (typeof scrollableNode === 'number') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this also work as intended on Fabric where there are shadow node wrappers instead of view tags (numbers)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On both platforms, getScrollableNode returns a number - I've checked it for both ScrollView and FlatList.

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;
piaskowyk marked this conversation as resolved.
Show resolved Hide resolved
}

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