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

Revert "Remove setNativeProps API" change

Open niklasmachacek opened this issue 1 year ago • 4 comments

Is there an existing request?

  • [X] I have searched for this request

Describe the feature request

Back in March last year, the setNativeProps API was removed, since back then the react-native team said that the API would not be supported in Fabric.

This changed as of now as it can be read here.

It would only make sense to get setNativeProps API back into react-native-web again since many people are relying on the API (for example for text input animations) and it is now officially supported in Fabric after react 0.72.

There is also a discussion about this here, where many people agree that this would be the best change.

niklasmachacek avatar Jan 25 '24 13:01 niklasmachacek

setNativeProps never worked correctly on web and had undefined behaviour on native. If the fabric implementation has defined behaviour, there's still no reliable way to implement it on web while styles are not simply inline styles. The primary use case of setNativeProps was updating styles without a render, but it's just not reliable on web. Things like text input animations aren't done this way on web either. The new RN architecture will eventually allow for sync events and sync updates, providing the same event loop model as web, so the setNativeProps pattern will likely have fewer reasons to be used on native

necolas avatar Jan 25 '24 17:01 necolas

@necolas I appreciate the technical concerns here—thank you!—and I'm sure it's not easy to convey without the reader already having some context around the implementation.

From my point of view, setNativeProps in RNW (0.18) works well-enough for styles (which might be because of how styles work in RNW 0.18) but more importantly works for setting text input values without round-tripping via the React lifecycle. My use case is for things like formatted text inputs between focus/blur.

I have previously tried to remove setNativeProps in favour of the suggested state-based approaches, but I ran into update cascades/loops (certainly in the native code), and overall introduces more complexity and fragility than setNativeProps for these types of tasks. For style updates—such as handling hover—setNativeProps is perceptibly more responsive than the state round-trip.

Do you think there's some way forward where a "use at own risk" type API is introduced for setNativeProps for these difficult edge cases? Or is this a hard line from your point of view?

jonathanj avatar Jan 30 '24 11:01 jonathanj

https://github.com/facebook/react-native/commit/1d3fa40c59b234f21f516db85c322ec0ed0311e0

Add setNativeProps to Fabric

Summary: changelog: Introduce setNativeProps to Fabric

Add support for setNativeProps in Fabric for backwards compatibility. It is still recommended to move away from setNativeProps because the API will not work with future features.

We can make step Migrating off setNativeProps in migration guide optional.

Reviewed By: yungsters, mdvacca

Differential Revision: D41521523

fbshipit-source-id: 4d9bbd6304b8c5ee24a36b33039ed33ae1fc21f8

dalcib avatar Jan 30 '24 14:01 dalcib

@jonathanj For now I think you could use Platform.OS forks to use setNativeProps on native and direct manipulation on web. Eventually RN will support the same direct manipulation APIs as web, and will support sync updates that make things like managing text input content better than it is today.

necolas avatar Jan 30 '24 19:01 necolas