Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

after change to ver 2.0.0 #31

Closed
Tkoefod opened this issue Feb 2, 2017 · 12 comments
Closed

after change to ver 2.0.0 #31

Tkoefod opened this issue Feb 2, 2017 · 12 comments

Comments

@Tkoefod
Copy link

Tkoefod commented Feb 2, 2017

when trying to change to the new version all I get is an error on some length as follows:

Cannot read property 'length' of undefined
handleException @ ExceptionsManager.js:63
handleError @ InitializeCore.js:114
reportFatalError @ error-guard.js:44
guard @ MessageQueue.js:48
callFunctionReturnFlushedQueue @ MessageQueue.js:107
(anonymous) @ debuggerWorker.js:71
ExceptionsManager.js:71 Unhandled JS Exception: Cannot read property 'length' of undefined

@bd-arc
Copy link
Contributor

bd-arc commented Feb 3, 2017

@Tkoefod Hi there! Have you read about the breaking change introduced with version 2.0.0?
Could you provide some code for us to check?

@Tkoefod
Copy link
Author

Tkoefod commented Feb 3, 2017

Yes I did read about the breaking changes, and now it works but only if I delay loading your component until after the init render for some reason, but at least it works now I just don't run the component until the componentDidMount execs. I took your component back all the way to basics and it still wouldn't run for me before, my guess without looking into your code much is something about my layout that it depends on for sizing? Anyhow great component otherwise and I love it.

@bd-arc
Copy link
Contributor

bd-arc commented Feb 3, 2017

@Tkoefod You are fetching some data, aren't you? My guess is that you're trying to render the carousel while it has no child at all. Am I right? Indeed, this would fail since the component depends on children's length.

I would recommend you to render the carousel conditionally. Something along those lines:

get loader () {
    return (
        <ActivityIndicator size={'small'} />
    );
}

get carousel () {
    const { entries } = this.state;
    return (
        <Carousel>
            { entries }
        </Carousel>
    );
}

render () {
    const { dataFetched } = this.state;
    return !dataFetched ? this.loader : this.carousel;
}

Do not hesitate to share some code if this doesn't do the trick!

@JakeRawr
Copy link
Contributor

JakeRawr commented Feb 3, 2017

I believe this is the same bug.
After I upgrade, I'm getting

undefined is not an object (evaluating 'interpolators.length')

My Code is very simple

<Carousel
  ref={'carousel'}
  firstItem={2}
  showsHorizontalScrollIndicator={false}
  sliderWidth={ Dimensions.get('window').width }
  itemWidth={GAME_COVER_WIDTH}
  onSnapToItem={(currentIndex) => this.updateGameIndex(currentIndex)}
>
  <View><Text>{'HI 1'}</Text></View>
  <View><Text>{'HI 2'}</Text></View>
</Carousel>

@Jonarod
Copy link
Contributor

Jonarod commented Feb 3, 2017

@JakeRawr Have you tried without the onSnapToItem prop ?
I believe this callback can get a bit funky. I don't think this is the idiomatic approach, but I kinda solved a similar issue by enclosing the function into a setTimeout() like this:

onSnapToItem={ (currentIndex) => setTimeout( () => { this.updateGameIndex(currentIndex) }, 1) }

Could you give it a try ?

@bd-arc In fact, with momentum disabled, I already faced some troubles with onSnapToItem() callback: when calling it for say setting a new state, the scroll animation is quite funky as it scrolls like if momentum was enabled (i.e depending on the velocity I use with my thumb, going at index number 10 for example) then suddenly jumps to desired index... the scroll animation feels something like : action => "from index 0, go to 1", result => "from index 0, go to index 10, come back to index 1". Then, after enclosing the callback into a setTimeout() like exposed, the behavior is just like desired...

Maybe there is something to do in the snapToItem() core function so that this callback effectively gets called AFTER scroll to the new position.

@JakeRawr
Copy link
Contributor

JakeRawr commented Feb 3, 2017

@Jonarod Thanks for getting back to me so quickly

removing onSnapToItem still doesn't do the trick. I've put a timeout to load the Carousel.

I'm going to take some time and run the exact code from the example folder. I may be missing something.

@bd-arc
Copy link
Contributor

bd-arc commented Feb 6, 2017

@JakeRawr I can see that your carousel has two children but you've set firstItem to 2. Since this prop is index-based, it should be 0 or 1. Can you try that and get back to me?

@Jonarod Thank you for your investigation. You're absolutely right: onSnaptoItem() shouldn't be called before the scrolling animation is finished. We'll look into it as soon as possible :)

@bd-arc
Copy link
Contributor

bd-arc commented Feb 6, 2017

@Jonarod Well, this isn't going to be a walk in the park. I've created a dedicated issue so that everyone can find out why and provide ideas...

@chitezh
Copy link
Contributor

chitezh commented Feb 11, 2017

@JakeRawr @bd-arc The problem is with un-handled exception in componentWillReceiveProps.

    componentWillReceiveProps (nextProps) {
        const { firstItem } = nextProps;
        const { interpolators } = this.state;

        if (interpolators.length !== nextProps.children.length) {
            this._positions = [];
            this._calcCardPositions(nextProps);
            this._initInterpolators(nextProps);
            this.setState({ activeItem: firstItem });
        }
    }

Two possibile solutions.

  • Initialize this.state.interpolators in the constructor
     this.state = {
            activeItem: props.firstItem,
+          interpolators: [],
        };
  • Defend against undefined interpolators
    componentWillReceiveProps (nextProps) {
        const { firstItem } = nextProps;
        const { interpolators } = this.state;

-        if (interpolators.length !== nextProps.children.length) {
+        if (interpolators && interpolators.length !== nextProps.children.length) {
            this._positions = [];
            this._calcCardPositions(nextProps);
            this._initInterpolators(nextProps);
            this.setState({ activeItem: firstItem });
        }
    }

@chitezh
Copy link
Contributor

chitezh commented Feb 11, 2017

I could submit a PR if you want

@bd-arc
Copy link
Contributor

bd-arc commented Feb 13, 2017

@chitezh Thank you very much for the PR! This has been merged and published ;)

@bd-arc bd-arc closed this as completed Feb 13, 2017
@JakeRawr
Copy link
Contributor

Thank you guys for fixing this! I've gotten it to work. I do have another problem with enableMomentum but create another issue to discuss the problem.

rosslavni pushed a commit to rosslavni/react-native-snap-carousel that referenced this issue Aug 11, 2022
james-frontend pushed a commit to james-frontend/react-native-snap-carousel that referenced this issue Sep 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants