search

callstack / react-native-pager-view

React Native wrapper for the Android ViewPager and iOS UIPageViewController.
MIT License
2.77k stars 422 forks source link

Review recycler issue inside the pager #819

Open troZee opened 7 months ago

troZee commented 7 months ago

There might be an issue inside the pagerview. Please checkout the below link:

https://github.com/facebook/react-native/issues/42732

cipolleschi commented 5 months ago

Yep, the problem here is that pager-view is using a view provided by fabric as content-view. That should not be done as UIPageViewController modifies the content view and, if then Fabric recycle it, it might serve a modified view to some other surface.

Instead of init the pager view with a React Native view, please consider adding the React Native view as subview of the UIKit view, so that we do not risk to recycle the UIKit view.

I can share more details if needed!

troZee commented 5 months ago

Thank you @cipolleschi and @j-piasecki for checking this issue 🙏

I can share more details if needed!

So I have a question to clarify that. Do you mean to wrap every react native children view with UIView, so UIPageViewController will not interfere with react native children view?

cipolleschi commented 5 months ago

From my exploration, there is this extension that defines an initWithView, that is used here and here to create a ViewController where the root view is a RCTView.

The problem with this approach is that the root view of this ViewController, once set as a child of a UIPagerViewController, is modified by UIKit, setting some native properties that Fabric can't reset automatically, because it doesn't know that they have been changed.

Instead of doing that, we should create a regular ViewController and set the RCTView as a subview of the default view of the view controller. So that UIKit does not modify the view coming from fabric.

Does it make more sense?