[iOS] Fabric: make built-in components recycle optional

[zhongwuzw]

Summary:

Fixes #42732. We already support custom component view recycle optional, but seems we have no interface to disable built-in components recycling.

Changelog:

[IOS] [ADDED] - Fabric: make built-in components recycle optional

Test Plan:

To disable built-in components recycle, we can call like [*ComponentView setShouldBeRecycled:false].

[javache]

Let's not use macros and static variables here. Recycling should be configured as part of a class's implementation, by overriding shouldBeRecycled.

I think we should consider changing the default to false to make sure components are aware of the need to reset props in between changes, but that's also why you should compare _props and props in the updateProps method.

[sammy-SC]

I don't think disabling recycling on core components is a good idea. We depend on this to achieve better performance characteristics and have plans around perf that depend on it.

Regarding https://github.com/facebook/react-native/issues/42732, manually sizing a RCTViewComponentView will cause bugs. If you size component manually, bypassing the shadow tree, React Native will not have visibility into size of the view and hit testing won't work.

[zhongwuzw]

I don't think disabling recycling on core components is a good idea. We depend on this to achieve better performance characteristics and have plans around perf that depend on it.

Regarding #42732, manually sizing a RCTViewComponentView will cause bugs. If you size component manually, bypassing the shadow tree, React Native will not have visibility into size of the view and hit testing won't work.

@sammy-SC Yes, recycling is very critical for performance. But seems we don't have an easy way to add some custom recycle cleanup codes of built-in core components for users, if users add some custom logic on core components, they can do some cleanup when recycled.

cc. @j-piasecki Any thoughts about the fix of your issue? :)

[j-piasecki]

I don't think that disabling the recycling for a particular view type is the solution, especially when the view in question is RCTViewComponentView.

if users add some custom logic on core components, they can do some cleanup when recycled.

That's the way it should be done, I've opened the issue mainly to raise the point that it's not always done directly - like in the linked react-native-pager-view where calling setViewControllers causes the platform itself to change the autosizing mask of the view. Hopefully, finding the issue will save some time for people with a similar problem.

Any thoughts about the fix of your issue? :)

At the moment we patched RCTViewComponentView to add self.autoresizingMask = UIViewAutoresizingNone; inside prepareForRecycle and it fixed the problem for us. I don't know whether that would be an acceptable solution to include in the core of RN.

[sammy-SC]

I would be more inclined to reset autoresizingMask to none, rather than disabling view recycling.

calling setViewControllers causes the platform itself to change the autosizing mask of the view

This is interesting. So Calling setViewControllers (which takes an array of view controllers I suppose) will cause the UIPageViewController to change autoresizingMask on a view? Does it call it always on the top most view? If that's the case, we can override prepareForRecycle in RCTRootComponentView and reset autoresizingMask there.

[j-piasecki]

This is interesting. So Calling setViewControllers (which takes an array of view controllers I suppose) will cause the UIPageViewController to change autoresizingMask on a view?

It looks this way from the stack trace we got from overriding the setter for the autoresizingMask in the RCTViewComponentView. It's kind of hard to tell what happening there exactly through the assembly 😅.

[react-native-bot]

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[cipolleschi]

We can close it. The problem was surfaced because of a flaw in the original library and we don't want to provide this functionality to components as it is implemented here.