[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].

[cipolleschi]

I don't think this is the right approach because this is a static method. It means that it applies to ALL the components of a given type.
So, let's imagine that we set RCTComponentViewShouldBeRecycled(false) to RCTView, we are disabling the recyclability for ALL the RCTViews.
The possibility to be recycled or not should be a choice made instance per instance.

I should be able to say that my RCTImage x should not be recycled (perhaps it is a large image and I want to keep it i memory as a perf optimization, while my RCTImage y should not.

What do you think?

cc. @sammy-SC which knows this part of the Framework better than me.

[zhongwuzw]

Yeah, I just follow the custom user components because it's class method implemented in

react-native/packages/react-native/React/Fabric/Mounting/RCTComponentViewFactory.mm

Line 99 in ebb55a7

.shouldBeRecycled = [viewClass respondsToSelector:@selector(shouldBeRecycled)]

Maybe we can expose some like a delegate to AppDelegate and give its instance to ask the user to decide whether to disable recycle?

[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 #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.