Skip to content

Add displayName to forwardRef from AnimatedComponent #6978

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Feb 11, 2025
Merged

Add displayName to forwardRef from AnimatedComponent #6978

merged 5 commits into from
Feb 11, 2025

Conversation

m-bert
Copy link
Contributor

@m-bert m-bert commented Feb 4, 2025 •
edited by github-actions bot
Loading

Summary

After switching to new findNodeHandle implementation based on display: contents;, we can no longer detect components from SVG that are wrapped using createAnimatedComponent (see issue on Gesture Handler side). This is because our Wrap receives forwardRef as child - that makes it impossible to detect what component is used inside. Theoretically we could do something like

child.type.render().displayName

and use regex to get component name, but it would be very expensive.

Instead, we can add displayName directly to forwardRef on Reanimated side. This should help us detect if component passed inside createAnimatedComponent was from SVG.

Test plan

Code used inside our Wrap.web.tsx

const child = React.Children.only(children);

console.log(child)
console.log(child.type.displayName)

gives the following result:

image

@m-bert m-bert requested review from piaskowyk and MatiPl01 February 4, 2025 15:16
@m-bert m-bert mentioned this pull request Feb 5, 2025
Merged
@tjzel tjzel assigned tjzel and unassigned tjzel Feb 7, 2025
@tjzel tjzel self-requested a review February 7, 2025 15:40
tomekzaw
tomekzaw approved these changes Feb 10, 2025
View reviewed changes
Copy link
Member

@tomekzaw tomekzaw left a comment

Choose a reason for hiding this comment

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

LGTM, haven't tested it though

@m-bert m-bert added this pull request to the merge queue Feb 11, 2025
Merged via the queue into main with commit b5cf1b7 Feb 11, 2025
9 checks passed
@m-bert m-bert deleted the @mbert/add-display-name branch February 11, 2025 11:41
@m-bert m-bert mentioned this pull request Feb 11, 2025
Closed
m-bert added a commit to software-mansion/react-native-gesture-handler that referenced this pull request Feb 12, 2025
@m-bert
[Web] Handle SVG wrapped with createAnimatedComponent (#3379)
bb248de 
## Description

Currently, when user wraps `SVG` component with `createAnimatedComponent`, we have no way to detect that `child` node passed into [Wrap](https://github.com/software-mansion/react-native-gesture-handler/blob/6e8d88e5a3b15bb399c188d4c23603b4e0f4a8bc/src/handlers/gestures/GestureDetector/Wrap.web.tsx#L9) comes from `SVG`. This results in additional `div` with `display" contents;` being added, which effectively breaks `SVG`.

>[!CAUTION]
>In order to fix this, I've opened [PR against Reanimated](software-mansion/react-native-reanimated#6978) that adds inner component name into `displayName` of the `forwardRef` that `createAnimatedComponent` returns. 

Fixes #3356 

## Test plan

<details>
<summary>Tested on the code from issue</summary>

```jsx
import { useMemo, useRef } from 'react';
import {  Text, View, Animated, PanResponder } from "react-native";
import { Svg, Rect } from 'react-native-svg';
import { GestureDetector, Gesture, GestureHandlerRootView } from 'react-native-gesture-handler';
import RNRAnimated, { useSharedValue, useAnimatedProps} from 'react-native-reanimated'

const RNRAnimatedRect = RNRAnimated.createAnimatedComponent(Rect)
const AnimatedRect = Animated.createAnimatedComponent(Rect)

export default function Index() {
  const pan = useRef(new Animated.Value(0)).current;
  const panResponder = useRef(
    PanResponder.create({
      onMoveShouldSetPanResponder: () => true,
      onPanResponderMove: Animated.event([null, {dx: pan}]),
      onPanResponderRelease: () => {
        pan.extractOffset();
      },
    }),
  ).current;

  const RNTest = <View style={{borderWidth: 1}}>
    <Text>With RN... though this isn't ideal because it lets you drag the yellow region</Text>

    <Animated.View {...panResponder.panHandlers}>
      <Svg width={200} height={100} >
        <Rect width={200} height={100} fill='yellow'/>
        <AnimatedRect width={100} height={100} x={pan} y={0} fill='red'/>
      </Svg>
    </Animated.View>
  </View>

  const x = useSharedValue(0)
  const animatedX = useAnimatedProps(() => ({x: x.value}), [x]);
  const xStart = useSharedValue(0);
  const svgPanGesture = useMemo(() => {
    return Gesture.Pan()
      .onBegin(() => {xStart.value = x.value})
      .onChange((e) => {
        x.value = xStart.value + e.translationX
      })
  }, [])

  const RNGHTest = <View style={{borderWidth: 1}}>
    <Text>With RNGH and RNR</Text>

    <Svg width={200} height={100}>
    <Rect width={200} height={100} fill='yellow'/>
      <GestureDetector gesture={svgPanGesture}>
        <RNRAnimatedRect width={100} height={100} animatedProps={animatedX} y={0} fill='red' />
      </GestureDetector>        
    </Svg>
  </View>

  return (
    <GestureHandlerRootView style={{ flex: 1 }}>
    <View
      style={{
        flex: 1,
        justifyContent: "center",
      }}
    >
      {RNTest}
      {RNGHTest}
    </View>
    </GestureHandlerRootView>
  );
}
```

</details>
m-bert added a commit that referenced this pull request Feb 18, 2025
@m-bert
Add displayName to forwardRef from AnimatedComponent (#6978)
0ca95e8 
After switching to new `findNodeHandle` implementation based on
`display: contents;`, we can no longer detect components from `SVG` that
are wrapped using `createAnimatedComponent` (see
[issue](software-mansion/react-native-gesture-handler#3356)
on Gesture Handler side). This is because our
[Wrap](https://github.com/software-mansion/react-native-gesture-handler/blob/7f90209add0b761d3738882339f4949b31f6f88f/src/handlers/gestures/GestureDetector/Wrap.web.tsx#L9)
receives `forwardRef` as child - that makes it impossible to detect what
component is used inside. Theoretically we could do something like

```js
child.type.render().displayName
```

and use regex to get component name, but it would be very expensive.

Instead, we can add `displayName` directly to `forwardRef` on Reanimated
side. This should help us detect if component passed inside
`createAnimatedComponent` was from `SVG`.

Code used inside our
[Wrap.web.tsx](https://github.com/software-mansion/react-native-gesture-handler/blob/7f90209add0b761d3738882339f4949b31f6f88f/src/handlers/gestures/GestureDetector/Wrap.web.tsx#L9)

```jsx
const child = React.Children.only(children);

console.log(child)
console.log(child.type.displayName)
```

gives the following result:

<img width="605" alt="image"
src="https://github.com/user-attachments/assets/e12059bb-ebbc-4812-8a1a-ae8eb6507cf2"
/>
@m-bert m-bert mentioned this pull request Feb 18, 2025
Merged
tjzel pushed a commit that referenced this pull request Feb 18, 2025
@m-bert @tjzel
Add displayName to forwardRef from AnimatedComponent (#6978)
27278d6 
After switching to new `findNodeHandle` implementation based on
`display: contents;`, we can no longer detect components from `SVG` that
are wrapped using `createAnimatedComponent` (see
[issue](software-mansion/react-native-gesture-handler#3356)
on Gesture Handler side). This is because our
[Wrap](https://github.com/software-mansion/react-native-gesture-handler/blob/7f90209add0b761d3738882339f4949b31f6f88f/src/handlers/gestures/GestureDetector/Wrap.web.tsx#L9)
receives `forwardRef` as child - that makes it impossible to detect what
component is used inside. Theoretically we could do something like

```js
child.type.render().displayName
```

and use regex to get component name, but it would be very expensive.

Instead, we can add `displayName` directly to `forwardRef` on Reanimated
side. This should help us detect if component passed inside
`createAnimatedComponent` was from `SVG`.

Code used inside our
[Wrap.web.tsx](https://github.com/software-mansion/react-native-gesture-handler/blob/7f90209add0b761d3738882339f4949b31f6f88f/src/handlers/gestures/GestureDetector/Wrap.web.tsx#L9)

```jsx
const child = React.Children.only(children);

console.log(child)
console.log(child.type.displayName)
```

gives the following result:

<img width="605" alt="image"
src="https://github.com/user-attachments/assets/e12059bb-ebbc-4812-8a1a-ae8eb6507cf2"
/>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomekzaw tomekzaw tomekzaw approved these changes

@piaskowyk piaskowyk piaskowyk approved these changes

@tjzel tjzel tjzel approved these changes

@MatiPl01 MatiPl01 Awaiting requested review from MatiPl01

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@m-bert @tomekzaw @piaskowyk @tjzel