react-native-web icon indicating copy to clipboard operation
react-native-web copied to clipboard

`setNativeProps` ignores previous style changes

Open piaskowyk opened this issue 3 years ago • 10 comments

The problem

Update react-native-web from 0.12.3 to 0.13.0 broke setNativeProps function, and next call setNativeProps override previously applayed style.

For example. I have component:

<View
  ref={viewRef}
  style={{ width: 100, height: 100, background: "#333" }}
/>

I want to change the style of the component. Operation:

viewRef.setNativeProps({ style: { width: 200 } });
viewRef.setNativeProps({ style: { height: 200 } })

Result in version 0.12.3: view style = { width: 200, height: 200 }

Result in version 0.13.0 (and newer): view style = { width: 100, height: 200 }

How to reproduce

Just run both apps

Example in version 0.12.3: https://codesandbox.io/s/rnw-12-y11uy Example in version 0.13.0: https://codesandbox.io/s/rnw-13-sbuto

Steps to reproduce:

  1. Run both examples
  2. Click a couple of times on the button
  3. Observe differences

Expected behavior

I expect behavior like was in version 0.12.3

Environment (include versions). Did this work in previous versions?

  • React Native for Web (version): 0.13.0
  • React (version): 17.01
  • Browser: 17.01

Additional context

In react-native-reanimated we use setNativeProps to apply changed style during an animation. But after upgrade react-native-web, we have strange issues on the web version if one animation is longer than another component back to the initial style.

Example issue:

  • https://github.com/software-mansion/react-native-reanimated/issues/1804
  • https://github.com/software-mansion/react-native-reanimated/issues/1450

I made a little investigation and confirmed that setNativeProps doesn't change the initial styles of component: https://github.com/necolas/react-native-web/blob/c47bec7b93d6a3b7c31bbc8bb2e4acd117b79bfc/packages/react-native-web/src/modules/usePlatformMethods/index.js#L29-L40

I can prepare PR to apply changes to the initial style, but I'm not sure how this can affect other components.

Thanks for any help!

piaskowyk avatar Mar 12 '21 17:03 piaskowyk

Sure, you can make a PR. Just be aware that the behavior of setNativeProps is undefined in RN and complicated to implement on web.

necolas avatar Mar 12 '21 19:03 necolas

This patch is available in 0.15.2

necolas avatar Mar 29 '21 18:03 necolas

c0abdbfaaead4b6fd999cb767cfa1de1cc046564

necolas avatar Mar 29 '21 18:03 necolas

Thank you both for the quick fix here.

nandorojo avatar Mar 29 '21 18:03 nandorojo

Re-opening as the patch caused a regression and has been reverted in 0.15.7

necolas avatar Apr 13 '21 19:04 necolas

This will be very hard if not impossible to match react native. The good news is that this API will eventually be deprecated and removed from RN. The bad news is that the way React DOM updates, combined with the way RN style is converted to RDOM className + style, means that React DOM updates won't always clear away styles set using setNativeProps. Ideally an API like this would be integrated with the platform renderer. Same is true for touching the DOM outside of render, e.g., during style injection. I've brought this up many times with the React team but there is no progress because they're all focused on trying to get concurrent mode out.

Ultimately, I think React DOM is not the ideal renderer, but we need to use it for compat with the existing ecosystem. One day we might be able to build a new renderer with no backwards compat for React DOM and it would be a lot more efficient and powerful

necolas avatar Apr 16 '21 19:04 necolas

@piaskowyk can you think of an alternate solution for Reanimated, since it sounds unlikely that this patch will be in RNW?

Currently, Reanimated only functions properly with RNW 0.15.2 - 0.15.6. This is difficult for web adoption.

Could reanimated internally keep track of the previous value it passed to setNativeProps?

nandorojo avatar Sep 04 '21 15:09 nandorojo

Reanimated now has an updated version that sort of solves this internally. So you could probably close this issue.

However, Reanimated on Web still has its occasional bugs, namely for springs or styles that update after a component gets unmounted/remounted.

It's possible these are bugs I should open on the Reanimated repo. But they feel more like race conditions that come about from the use of setNativeProps.

Given the limitations of setNativeProps (and the plans to deprecate it), would it be a bad idea to directly manipulate the DOM node's style inside of Reanimated, rather than use setNativeProps?

RNW exposes the DOM ref, so I figured this might be a better option. Reanimated is hugely a popular library whose internals are very low-level, so it seems like an appropriate use-case to go around RNW.

I'm asking a bit naïvely, since there may be some obvious reasons not to do that.

nandorojo avatar Dec 05 '21 19:12 nandorojo

I'm on Reanimated v2.2.4, react-native-web v0.17.5 and I still get an error:

TypeError
Cannot read property 'previousStyle' of undefined
Screen Shot 2021-12-06 at 9 23 31 AM

Anything obvious I'm missing? 😅

elan avatar Dec 06 '21 19:12 elan

Yeah, use 2.2.3. 2.2.4 is broken on web I think.

nandorojo avatar Dec 06 '21 19:12 nandorojo