leecade / react-native-swiper

The best Swiper component for React Native.
MIT License
10.41k stars 2.34k forks source link

onIndexChanged produces Warning: Cannot update a component from inside the function body of a different component. #1209

Open mike-hamilton opened 4 years ago

mike-hamilton commented 4 years ago

Which OS ?

iOS

Version

Which versions are you using:

Expected behaviour

No errors/warnings?

Actual behaviour

This code produces a warning, Warning: Cannot update a component from inside the function body of a different component..

import React, {useState} from 'react';

import Swiper from 'react-native-swiper';

const [swiperIndex, setSwiperIndex] = useState(0);

const testComponent = () => {
  return <Swiper style={styles.wrapper} onIndexChanged={(i) => setSwiperIndex(i)}>
}

export default testComponent;
billel-boudchicha commented 4 years ago

Same problem here, this is cased by react being updated to 16.13.0

mike-hamilton commented 4 years ago

Same problem here, this is cased by react being updated to 16.13.0

This blog post about 16.13.0 says to wrap the setState call into useEffect. I'm not quite sure exactly what they mean or how to do that..

billel-boudchicha commented 4 years ago

I have the same use case I need to track the index of the swiper , but I think setSwiperIndex, should be wrapped in useEffect call, but this has to be in the Swiper component. Because the call of setSwiperIndex is inside the body of the Swiper component (if I'm not wrong)

zabojad commented 4 years ago

I'm having the same issue after having updated react-native-swiper to its latest git commit. I'm not using hooks in my app at all. I have only class components. @billel-boudchicha is there any PR for that already?

annguyen115 commented 4 years ago

any update on this issue ?

iquirino commented 4 years ago

Same issue here... :'(

CDBridger commented 4 years ago

Apparently its this here that causes it on line 213

UNSAFE_componentWillUpdate(nextProps, nextState) {
    // If the index has changed, we notify the parent via the onIndexChanged callback
    if (this.state.index !== nextState.index)
      this.props.onIndexChanged(nextState.index)
  }
dbenfouzari commented 4 years ago

I think React wants react-native-swiper to do something like

useEffect(() => {
  this.props.onIndexChanged && this.props.onIndexChanged(index)
}, [index])

But we have no control over it.

The library seems to be old, the code is written in an old fashioned way. Maybe it should be rewritten in a new React way.

jbreuer95 commented 4 years ago

Until there is a fix, you can do this nasty thing:

index.js

import { AppRegistry, LogBox } from 'react-native';

LogBox.ignoreLogs([
  'Warning: Cannot update a component from inside the function body of a different component',
]);
dbenfouzari commented 4 years ago

Ugh, I just went away from this package and used react-native-snap-carousel instead. Sorry guys.

jbromberg commented 4 years ago

Having the same issue.

yewenxiang23 commented 4 years ago

Having the same issue.

vietdiemtran commented 4 years ago

Has this error been fixed yet

dbenfouzari commented 4 years ago

@vietdiemtran I recommend to use react-native-snap-carousel instead. It's easy to use, and it works.

Aryk commented 4 years ago

@vietdiemtran I recommend to use react-native-snap-carousel instead. It's easy to use, and it works.

How to I give like 100 likes? 🥇

Thanks for the recommendation...good library, well abstracted and seems relatively stable.

milennaj commented 3 years ago

Is there a solution for this problem? Or can we get an alternative how to get the selected index that not includes switching to another control?

Thank you

dbenfouzari commented 3 years ago

Is there a solution for this problem? Or can we get an alternative how to get the selected index that not includes switching to another control?

Thank you

The only "good" solution was to switch to another library. Really, I did not managed to make it work properly

milennaj commented 3 years ago

Is there a solution for this problem? Or can we get an alternative how to get the selected index that not includes switching to another control? Thank you

The only "good" solution was to switch to another library. Really, I did not managed to make it work properly

Unfortunately control react-native-snap-carousel also has its problems, one is that on iOS refresh ScrollView doesn't work, so we lose a functionality to pull down for refresh. I would still like a response what is done about this issue? This is a basic functionality of the control that produces a warning (to just track selected index and use onIndexChange to set the state that tracks that value). It seems very bad that something so basic doesn't work.

Thanks

ManhVuTien commented 3 years ago

I use onIndexChanged={(newIndex) => { setTimeout(() => { setIndex(newIndex); }, 1); }} The warning is gone.

CDBridger commented 3 years ago

this fix is simple, I would do a PR myself but I no longer use this library as I ended up writing my own solution using a functional component which depends on a flatlist rather than using a scroll view. All that needs to be done to fix this is remove

UNSAFE_componentWillUpdate(nextProps, nextState) {
    // If the index has changed, we notify the parent via the onIndexChanged callback
    if (this.state.index !== nextState.index)
      this.props.onIndexChanged(nextState.index)
  }

and add

this.propsonIndexChanged(index)

to the updateIndex function between line 519 and 520

hungdev commented 3 years ago

you can use useRef instead of state to store indexStep value

import React, { useState, useRef } from 'react'
import { View, Text, Image } from 'react-native'
// import { translate } from 'i18n'
import styles from "./styles";
import { translate } from '../../i18n';
import { Images } from 'themes'
import Slide1 from "./Slide1";
import Slide2 from "./Slide2";
import Slide3 from "./Slide3";
import Slide4 from "./Slide4";
import Swiper from 'react-native-swiper'

const TOTAL_NUMBER_OF_PAGES = 4

export default function Onboard() {
  const swiperRef = useRef()
  const currentStep = useRef(0);

  const onNext = () => {
    if (currentStep.current < TOTAL_NUMBER_OF_PAGES - 1) { 
      swiperRef?.current?.scrollBy(1); // go forward one page
      currentStep.current += 1
    } else {
      // navigation.navigate('Next Screen through navigation')
    }
  }

  const onIndexChanged = (ind) => {
    currentStep.current = ind
  }
  return (
    <View style={styles.container}>
      <Swiper
        style={styles.wrapper}
        showsButtons
        loop={false}
        ref={swiperRef}
        onIndexChanged={onIndexChanged}
      >
        <Slide1 onNext={onNext} />
        <Slide2 onNext={onNext} />
        <Slide3 onNext={onNext} />
        <Slide4 onNext={onNext} />
      </Swiper>
    </View>
  )
}
MrPocketsss commented 3 years ago

I got this to work by modifying a bit of the code. All of the UNSAFE_component stuff will be deprecated soon, so I've been toying with the Swiper code to get it fixed. To address this specific issue, remove:

UNSAFE_componentWillUpdate(nextProps, nextState) {
    // If the index has changed, we notify the parent via the onIndexChanged callback
    if (this.state.index !== nextState.index)
      this.props.onIndexChanged(nextState.index)
  }

and replace componentDidUpdate with:

componentDidUpdate(prevProps) {
    // If autoplay props updated to true, autoplay immediately
    if (this.props.autoplay && !prevProps.autoplay) {
      this.autoplay();
    }
    if (this.props.children !== prevProps.children) {
      if (this.props.loadMinimal && Platform.OS === 'ios') {
        this.setState({ ...this.props, index: this.state.index });
      } else {
        this.setState(this.initState({ ...this.props, index: this.state.index }, true));
      }
    }
    // if the index has changed, we notify the parent via the onIndexChanged callback
    this.props.onIndexChanged(this.state.index);
  }
FabianTauriello commented 3 years ago

Any solution for this yet?

hungdev commented 3 years ago

@FabianTauriello switch to react-native-snap-carousel instead.

alena424 commented 3 years ago

Just use setTimeout:

const updateIndex = React.useCallback((res) => {
        setTimeout(() => {
            setSelectedTab(res)
        })
    }, [])
trungls1706 commented 3 years ago

in my case, i fix by way onIndexChanged={(index) => setTimeout(() => { setIndex(index) }, 0) }