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

Migrate away from setNativeProps on web #4352

Merged
merged 26 commits into from
May 16, 2023

Conversation

tomekzaw
Copy link
Member

@tomekzaw tomekzaw commented Apr 11, 2023

Summary

This draft PR moves off of setNativeProps in favor of editing DOM nodes directly.

Closes #4335, related to #2756.

Test plan

App.tsx
import Animated, {
  useSharedValue,
  withTiming,
  useAnimatedStyle,
  cancelAnimation,
  withRepeat,
  interpolateColor,
} from 'react-native-reanimated';
import { View, StyleSheet } from 'react-native';
import React, { useEffect } from 'react';

export default function AnimatedStyleUpdateExample(): React.ReactElement {
  const randomWidth = useSharedValue(0);

  useEffect(() => {
    cancelAnimation(randomWidth);
    randomWidth.value = withRepeat(withTiming(1), -1, true);
  }, [randomWidth]);

  const style = useAnimatedStyle(() => {
    return {
      width: randomWidth.value * 100,
      height: randomWidth.value * 100,
      backgroundColor: interpolateColor(
        randomWidth.value,
        [0, 1],
        ['red', 'lime']
      ),
      transform: [{ rotate: `${randomWidth.value * 180}deg` }],
    };
  });

  return (
    <View style={styles.container}>
      <Animated.View style={style} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    alignItems: 'center',
    justifyContent: 'center',
  },
});
Nagranie.z.ekranu.2023-05-15.o.13.45.19.mov

@chrisedington
Copy link

Thank you for this @tomekzaw

@macrozone
Copy link

thx @tomekzaw , is there an easy way to test this in moti?

would love to get this to work in nextjs!

@nandorojo
Copy link
Contributor

If you use patch-package, you can test it by editing this reanimated file in your node_modules: https://github.com/software-mansion/react-native-reanimated/pull/4352/files#diff-c91126f95c199aee12f3e0909df9a49ccb8bfd7c529a1430446df596e2c71330

Then yarn patch-package react-native-reanimated and yarn next

@iwash0120
Copy link

iwash0120 commented Apr 29, 2023

@tomekzaw Thank you!
Does this support transforms?
I tried but could not use it.

EDITED:
Sorry, I changed it a bit and was able to handle it.

import createReactDOMStyle from 'react-native-web/dist/exports/StyleSheet/compiler/createReactDOMStyle';
// Added.
import { createTransformValue } from 'react-native-web/dist/exports/StyleSheet/preprocess';

...

const domStyle = createReactDOMStyle(currentStyle);
for (const key in domStyle) {
  (component.style as StyleProps)[key] = domStyle[key];
}
// Added.
if (currentStyle.transform) {
  (component.style as StyleProp).transform = createTransformValue(currentStyle.transform);
}
```

component.setNativeProps({ style: currentStyle });

const domStyle = createReactDOMStyle(currentStyle);
for (const key in domStyle) {
Copy link

@alantoa alantoa May 2, 2023

Choose a reason for hiding this comment

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

We should handle this situation specifically because react-native-web has already removed array transform. You can check it out here.

Something just like this:

import { createTransformValue } from 'react-native-web/dist/exports/StyleSheet/preprocess';

for (const key in domStyle) {
  if(key === 'transform') {
      (component.style).transform = createTransformValue(domStyle[key]);
  }else{
      (component.style)[key] = domStyle[key];
  }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

does the current RNW function not account for it?

Copy link

Choose a reason for hiding this comment

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

Uunfortunately no. If you don't use createTransformValue, useAnimatedStyle will not work.

@tomekzaw
Copy link
Member Author

tomekzaw commented May 8, 2023

Thanks @y120sb and @alantoa, added transform array support in 551f37a

Copy link
Member

@piaskowyk piaskowyk left a comment

Choose a reason for hiding this comment

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

In overall looks great 😊
I just posted some questions just to be sure.

jest.config.js Outdated Show resolved Hide resolved
src/reanimated2/core.ts Outdated Show resolved Hide resolved
src/reanimated2/js-reanimated/index.ts Show resolved Hide resolved
src/reanimated2/js-reanimated/index.ts Outdated Show resolved Hide resolved
WebExample/App.tsx Outdated Show resolved Hide resolved
@tomekzaw tomekzaw marked this pull request as ready for review May 16, 2023 15:19
@tomekzaw tomekzaw added this pull request to the merge queue May 16, 2023
Merged via the queue into main with commit 398e620 May 16, 2023
13 checks passed
@tomekzaw tomekzaw deleted the @tomekzaw/migrate-off-set-native-props branch May 16, 2023 16:28
@chrisedington
Copy link

Thanks @tomekzaw for the efforts.

tomekzaw added a commit that referenced this pull request May 18, 2023
## Summary

Smol fix after #4352 because `react-native-web` may not be installed.

## Test plan

<!-- Provide a minimal but complete code snippet that can be used to
test out this change along with instructions how to run it and a
description of the expected behavior. -->
fluiddot pushed a commit to wordpress-mobile/react-native-reanimated that referenced this pull request Jun 5, 2023
## Summary

This draft PR moves off of `setNativeProps` in favor of editing DOM
nodes directly.

Closes software-mansion#4335, related to software-mansion#2756.

## Test plan

<details>
<summary>App.tsx</summary>

```tsx
import Animated, {
  useSharedValue,
  withTiming,
  useAnimatedStyle,
  cancelAnimation,
  withRepeat,
  interpolateColor,
} from 'react-native-reanimated';
import { View, StyleSheet } from 'react-native';
import React, { useEffect } from 'react';

export default function AnimatedStyleUpdateExample(): React.ReactElement {
  const randomWidth = useSharedValue(0);

  useEffect(() => {
    cancelAnimation(randomWidth);
    randomWidth.value = withRepeat(withTiming(1), -1, true);
  }, [randomWidth]);

  const style = useAnimatedStyle(() => {
    return {
      width: randomWidth.value * 100,
      height: randomWidth.value * 100,
      backgroundColor: interpolateColor(
        randomWidth.value,
        [0, 1],
        ['red', 'lime']
      ),
      transform: [{ rotate: `${randomWidth.value * 180}deg` }],
    };
  });

  return (
    <View style={styles.container}>
      <Animated.View style={style} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    alignItems: 'center',
    justifyContent: 'center',
  },
});
```

</details>


https://github.com/software-mansion/react-native-reanimated/assets/20516055/93b8096f-2a4d-4c61-848d-f483d612ac13
fluiddot pushed a commit to wordpress-mobile/react-native-reanimated that referenced this pull request Jun 5, 2023
## Summary

Smol fix after software-mansion#4352 because `react-native-web` may not be installed.

## Test plan

<!-- Provide a minimal but complete code snippet that can be used to
test out this change along with instructions how to run it and a
description of the expected behavior. -->
import createReactDOMStyle from 'react-native-web/dist/exports/StyleSheet/compiler/createReactDOMStyle';

let createTransformValue: (transform: any) => any;
try {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be better to require this in the conditional itself, rather than at the root. I'm not sure if try catch requires work with Webpack and most parsers.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, you're right, already done in #4473.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Web implementation doesn't work in react-native-web 0.19
7 participants