[Discussion] setPage should not trigger onPageSelected event

[oliverdolgener]

Bug

We are storing and updating the active index of the ViewPager in Redux. When the user swipes to another page the onPageSelected event is correctly triggered and the active index will be updated like that:

<ViewPager
      ref={pager}
      pageMargin={0}
      onPageSelected={({ nativeEvent }) => setActiveIndex(nativeEvent.position)}
      style={[styles.pager, { width }]}
      initialPage={0}
>

But we also want to set the page programatically when the active index is changed by some other component. To achieve this we are using an useEffect hook like this:

useEffect(() => {
    pager.current && pager.current.setPageWithoutAnimation(activeIndex);
}, [activeIndex]);

Unfortunately calling setPageWithoutAnimation triggers another onPageSelected event which updates the active index which triggers the useEffect hook and so on.

We think that there should be no onPageSelected event triggered when setting the page programatically.

Environment info

React native info output:

System:
    OS: macOS Mojave 10.14.6
    CPU: (4) x64 Intel(R) Core(TM) i5-7360U CPU @ 2.30GHz
    Memory: 5.96 GB / 16.00 GB
    Shell: 3.2.57 - /bin/bash
  Binaries:
    Node: 10.16.3 - ~/.nvm/versions/node/v10.16.3/bin/node
    Yarn: 1.19.1 - /usr/local/bin/yarn
    npm: 6.13.1 - ~/.nvm/versions/node/v10.16.3/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  SDKs:
    iOS SDK:
      Platforms: iOS 13.2, DriverKit 19.0, macOS 10.15, tvOS 13.2, watchOS 6.1
    Android SDK:
      API Levels: 21, 22, 23, 24, 25, 26, 27, 28
      Build Tools: 23.0.1, 25.0.2, 25.0.3, 26.0.1, 26.0.2, 26.0.3, 27.0.3, 28.0.1, 28.0.3, 29.0.2
      System Images: android-19 | Google APIs Intel x86 Atom, android-21 | Google APIs Intel x86 Atom_64, android-22 | Intel x86 Atom_64, android-22 | Google APIs Intel x86 Atom_64, android-23 | Google APIs Intel x86 Atom_64, android-26 | Intel x86 Atom_64, android-27 | Google APIs Intel x86 Atom, android-28 | Google APIs Intel x86 Atom
      Android NDK: 20.1.5948944
  IDEs:
    Android Studio: 3.5 AI-191.8026.42.35.6010548
    Xcode: 11.3/11C29 - /usr/bin/xcodebuild
  npmPackages:
    react: 16.9.0 => 16.9.0
    react-native: 0.61.5 => 0.61.5

Library version: 3.3.0

[shanemduggan]

This would be useful to have.

[razzfox]

It's always going to cause an infinite loop when your dependency is the thing you are changing (activeIndex). It sounds like you are using one variable to reference two distinct values: the current state, and the desired state. It probably makes more sense to use a separate variable for the desired state (desiredIndex), and then you can check if (activeIndex !== desiredIndex) before you call setPage.

[razzfox]

What do you think about using a desiredIndex value as the useEffect dependency, so you have no infinite loop?

To be clear, I disagree with removing the onPageSelected event, because in this example, "current" and "desired" page values are not properly separated.

[oliverdolgener]

That's actually an interesting idea. We will give it a try.

[crystalneth]

I have the same issue. Distinguishing between desiredIndex and currentIndex isn't really possible, because we can't distinguish between a user initiated page scroll and programmatically initiated one, at least without some transitioning state variable.

You can see in the code below, if a user taps to change the segmented control index from 0 to 1, then back to 0 before the pager has finished transitioning to 1, then the onPageSelected event will fire with '1' before the user initiated transition back to 0 has started. An infinite loop ensues.

Note that unlike the above example I am using animations, which I do want.

Also note that the SegmentedControlIOS onChange event does not fire when its index is programmatically changed, however the ViewPager event does fire when its index is programmatically change. I believe the former is the correct behavior.

 const viewPager = React.createRef<ViewPager>()
 const [pageIndex, setPageIndex] = useState(0)

  useEffect(() => {
    console.log('useEffect setPage '+pageIndex)
    viewPager.current?.setPage(pageIndex)
  }, [pageIndex]);

...

        <SegmentedControlIOS
          selectedIndex={pageIndex}
          onChange={event => setPageIndex(event.nativeEvent.selectedSegmentIndex)}
        />
        <ViewPager initialPage={pageIndex} ref={viewPager} 
            onPageSelected={e => setPageIndex(e.nativeEvent.position)}
            >
...

[crystalneth]

@oliverdolgener Did you find an effective workaround?

[oliverdolgener]

@crystalneth not yet. what you describe is exactly our problem. I'll take a closer look this week and let you know.

[crystalneth]

I could not find a viable workaround to the event firing.

I later found more severe bugs in the firing of page selected events after adding a RN SectionList to a page. I was getting a pageselected event for every rendering of each list item, which caused the UI to lock up and go into infinite loops on initial render. Who knows why. Sure enough when I changed the number of list items it changed the number of event firings, and at zero list items it worked properly.

I've torn out this component and am now using react-navigation tab view. I was already using react-navigation (although I was considering react-native-navigation, and can no longer do that).

It's working well so far, and basically what I wanted except with a material design look instead of iOS, which I can live with (and also override if needed).

[troZee]

@crystalneth

I later found more severe bugs

Could you provide a example and create a separate issue for those bugs ?

[crystalneth]

I'm the sole dev on my project rn and have pulled the module out of the codebase, so I'm unlikely to get around to that. Sorry!

If you'd like to try what I had, it was two pages, one had a FlatList and the other a SectionList, and the interaction with the segmented controller is as specified above.

Sorry, I can't help more rn.

On Thu, Apr 16, 2020 at 12:03 AM troZee notifications@github.com wrote:

@crystalneth https://github.com/crystalneth

I later found more severe bugs

Could you provide a example and create a separate issue for those bugs ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/react-native-community/react-native-viewpager/issues/129#issuecomment-614454668, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAGFJMFKNFGIX3Y5SZMA5DRM2UULANCNFSM4JZ22DBA .

[ryanjwessel]

I am running into a similar problem. Issue #149 mentions this as well, but the examples provided only work if the selected index is only needed for ViewPager. When the active page is needed by multiple components and each of them can update the index, you will trigger an infinite loop.

In my example, I have two components that use this value. A Map component with MapMarkers, and a ViewPager that shows details for the selected MapMarker.

We always need to use onPageSelected to keep track of the selectedIndex. In our case, we are dispatching an action each time the user swipes to a page. This is so Map can update to select the same MapMarker. We dispatch the same action from the Map component when a MapMarker is selected.

When a user selects a MapMarker, that updates the selectedIndex. The ViewPager component receives a new prop, and we use that to call setPageWithoutAnimation. Because it triggers onPageSelected, it dispatches that same action again.

If we swipe to a page once, it calls onPageSelected, dispatches the action, props are updated, we use that to setPageWithoutAnimation, and it triggers onPageSelected again. If that was the only action the user took, then it works fine. We can compare the current and previous versions of the selectedIndex, and not call setPageWithoutAnimation since they are the same.

But, when the user swipes between two pages rapidly it dispatches two actions, A and B.

• Props are updated based on Action A, we call setPageWithoutAnimation, onPageSelected is triggered, and it dispatches Action A again.
• Props are updated based on Action B, we call setPageWithoutAnimation, onPageSelected is triggered, and it dispatches Action B again.
• Props are updated based on Action A, and since the selectedIndex from A is not equal to the selectedIndex from B we call setPageWithoutAnimation again. This goes on forever.

If there were an option to not trigger the event handler when we call one of the setPage methods, then all of this could be avoided. But, the current behavior makes it potentially impossible to reconcile state across multiple components.

[oliverdolgener]

@ryanjwessel Thanks man, this is exactly the problem that we have. I would appreciate a solution like you suggested

[ryanjwessel]

@oliverdolgener the only solution I found that actually kept the selectedIndex in-sync between multiple components was to provide a key prop to ViewPager that would force a re-render if the selectedIndex was changed by a component other than ViewPager. This came with significant performance drawbacks though, as it would re-render the entire ViewPager and it's children each time that happened. It would take several seconds to re-render with the correct index.

I am trying to figure out another solution now, but it is still in-progress!

[ryanjwessel]

@oliverdolgener So I think @razzfox's suggestion is the only way to accomplish this without completely re-rendering the component.

I added activeIndex as a component state field. The prop I receive from the Redux store is now the desiredIndex. If I receive new props and the desiredIndex does not match the activeIndex, then update activeIndex in state and as a part of your setState callback, you can execute setPageWithoutAnimation.

In your onPageSelected handler, you need to also check if activeIndex does not match desiredIndex (event.nativeEvent.position), before dispatching the action.

This appears to be working as expected.

[razzfox]

If I understand correctly, you are passing setActiveIndex to other components that need it, and you have a useEffect that depends on activeIndex, which calls setPageWithoutAnimation to change the page of the ViewPager.

If you pass setPageWithoutAnimation directly (or inside a helper function) to the other components, and they call that instead of setActiveIndex, would your useEffect still need to call setPageWithoutAnimation?

For example, these are the helper functions that I pass as props to child components (e.g. next/prev buttons):

const setPagePrev = () => (activeIndex > 0) && pager.current.setPage(activeIndex - 1)
const setPageNext = () => (activeIndex < slides.length - 1) && pager.current.setPage(activeIndex + 1)

[ryanjwessel]

Hey @razzfox, I'm not sure if this would work in my case. There are other components that are relying on this index being provided as a value. If we called setPageWithoutAnimation directly, say by pressing a MapMarker on a Map component, then there would be no way to indicate that the MapMarker is selected too.

This is because this component is pure React Native, so it makes a comparison between the selectedIndex and the MapMarker's index. Does that make sense?

If we were able to provide the selectedIndex directly to ViewPager then this value could be tracked purely in React Native/Javascript-land, but I don't know if it is possible given the native functionality for this package.

[inmogr]

setPage is not working at all for me the page is not being changed

[joyacv2]

Hi,

I also test this very simple code in iOS: onPageSelected={(e) => { console.log(e) }}

When doing movement between pages, the app start crashing aleatory without any error.

I am using the latest version : 4.0.1

[enagorny]

Ideally would be good to have react-native-viewpager as controlled component. Instead of initialPage might be better to have page which will control selected page.

[troZee]

Ideally would be good to have react-native-viewpager as controlled component. Instead of initialPage might be better to have page which will control selected page.

But in this solution, you have 2 source of truth which is state inside the component and the current native. So let me describe your solution for better understanding:

• VP has not got a onPageSelected callback
  • React component keeps state of VP current page
  • User swipes to the next page using gesture
  • Current page number is 2 and your state has not been updated

So the question is, how we should sync the current displayed page ? For now, it makes sense bc the state is updated in once callback and it does not matter, if you swipe programmatically or using a gesture

[enagorny]

So the question is, how we should sync the current displayed page ? • VP has not got a onPageSelected callback Without callback which will be executed after VP it wouldn't be possible.

For the controlled components https://reactjs.org/docs/forms.html#controlled-components will need some onChange callback which will update parent state usually.

[troZee]

Hello everyone, We have collected all yours suggestions and created a new PoC https://github.com/callstack/react-native-viewpager/pull/268

Feel free to comment

[jitenshah19]

@troZee : I am still facing issue where setPage triggers onPageSelected event in version 5.1.10 is there a work around that I can use ?

[y7haar]

@troZee is there any update on this issue? It seems that the pull request #268 is not merged yet. Any advice on how to use the new api would be great!

[tncbbthositg]

I still have the issue in 5.4.9. It looks like it's still going to exist in 6.0. In my case, I have the same PagerView component on multiple pages and a global state specifying (more or less) the selected index. I need a change on one screen to effect a change on all of the screens.

But, when the other screens switch pages programmatically following the global state change, they fire the onPageSelected. I was able to deal with this by not calling setPageWithoutAnimation if the global page index was the same as the current index.

But, as the app has grown and more work is being done, there's a race condition and the user is able to switch pages fast enough that the PagerView does get caught in an infinite loop and I can't do much about it because I can't tell when onPageSelected is being called due to user interaction or when it is being called because of a call to setPage*.

:(