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

Fix infinite loop in mappers #4429

Merged
merged 1 commit into from
May 9, 2023

Conversation

mstach60161
Copy link
Contributor

@mstach60161 mstach60161 commented May 4, 2023

Summary

In some cases when mappers trigger event and event trigger mapper, we can end up with infinite loop. To prevent it, I added if statement that checks if we are already running mappers.

Test plan

import React from 'react';

import Animated, {
  useAnimatedRef,
  withTiming,
  useAnimatedReaction,
  scrollTo,
  useSharedValue,
  useScrollViewOffset,
  useDerivedValue,
} from 'react-native-reanimated';
import { Button, StyleSheet, Text, View } from 'react-native';

export default function ScrollViewOffsetCallScrollToExample() {
  const aref = useAnimatedRef<Animated.ScrollView>();
  const sharedValue = useSharedValue(0);
  const scrollHandler = useScrollViewOffset(aref);

  const onChangeScrollValue = () => {
    sharedValue.value = Math.random() * 5000;
  };

  const onAnimatedChangeScrollValue = () => {
    sharedValue.value = withTiming(Math.random() * 5000);
  };

  useAnimatedReaction(
    () => {
      return sharedValue.value;
    },
    (value) => {
      scrollTo(aref, 0, value, false);
    }
  );

  useDerivedValue(() => {
    console.log(sharedValue.value)
    return {};
  });

  return (
    <>
      <View style={styles.positionView}>
        <Text>Test</Text>
        <Button
          title="Scroll without animation"
          onPress={onChangeScrollValue}
        />
        <Button
          title="Scroll with animation"
          onPress={onAnimatedChangeScrollValue}
        />
      </View>
      <View style={styles.divider} />
      <View style={styles.scrollsContainer}>
        <Animated.ScrollView
          ref={aref}
          style={styles.scrollView}>
          {[...Array(100)].map((_, i) => (
            <Text key={i} style={styles.text}>
              {i}
            </Text>
          ))}
        </Animated.ScrollView>
      </View>
    </>
  );
}

const styles = StyleSheet.create({
  positionView: {
    margin: 20,
    alignItems: 'center',
  },
  scrollsContainer: {
    flexDirection: 'row',
  },
  scrollView: {
    flex: 1,
    width: '100%',
  },
  text: {
    fontSize: 50,
    textAlign: 'center',
  },
  divider: {
    backgroundColor: 'black',
    height: 1,
  },
});

@mstach60161 mstach60161 added this pull request to the merge queue May 9, 2023
Merged via the queue into main with commit 7d2ee9b May 9, 2023
@mstach60161 mstach60161 deleted the @mstach60161/fix-infinite-loop-in-mappers branch May 9, 2023 10:38
fluiddot pushed a commit to wordpress-mobile/react-native-reanimated that referenced this pull request Jun 5, 2023
<!-- Thanks for submitting a pull request! We appreciate you spending
the time to work on these changes. Please follow the template so that
the reviewers can easily understand what the code changes affect. -->

## Summary

In some cases when mappers trigger event and event trigger mapper, we
can end up with infinite loop. To prevent it, I added if statement that
checks if we are already running mappers.

- fixes software-mansion#4405 

## Test plan

- Checked with example from this PR: software-mansion#4358 
- and code that caused that bug:

```
import React from 'react';

import Animated, {
  useAnimatedRef,
  withTiming,
  useAnimatedReaction,
  scrollTo,
  useSharedValue,
  useScrollViewOffset,
  useDerivedValue,
} from 'react-native-reanimated';
import { Button, StyleSheet, Text, View } from 'react-native';

export default function ScrollViewOffsetCallScrollToExample() {
  const aref = useAnimatedRef<Animated.ScrollView>();
  const sharedValue = useSharedValue(0);
  const scrollHandler = useScrollViewOffset(aref);

  const onChangeScrollValue = () => {
    sharedValue.value = Math.random() * 5000;
  };

  const onAnimatedChangeScrollValue = () => {
    sharedValue.value = withTiming(Math.random() * 5000);
  };

  useAnimatedReaction(
    () => {
      return sharedValue.value;
    },
    (value) => {
      scrollTo(aref, 0, value, false);
    }
  );

  useDerivedValue(() => {
    console.log(sharedValue.value)
    return {};
  });

  return (
    <>
      <View style={styles.positionView}>
        <Text>Test</Text>
        <Button
          title="Scroll without animation"
          onPress={onChangeScrollValue}
        />
        <Button
          title="Scroll with animation"
          onPress={onAnimatedChangeScrollValue}
        />
      </View>
      <View style={styles.divider} />
      <View style={styles.scrollsContainer}>
        <Animated.ScrollView
          ref={aref}
          style={styles.scrollView}>
          {[...Array(100)].map((_, i) => (
            <Text key={i} style={styles.text}>
              {i}
            </Text>
          ))}
        </Animated.ScrollView>
      </View>
    </>
  );
}

const styles = StyleSheet.create({
  positionView: {
    margin: 20,
    alignItems: 'center',
  },
  scrollsContainer: {
    flexDirection: 'row',
  },
  scrollView: {
    flex: 1,
    width: '100%',
  },
  text: {
    fontSize: 50,
    textAlign: 'center',
  },
  divider: {
    backgroundColor: 'black',
    height: 1,
  },
});

```
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

Successfully merging this pull request may close these issues.

[V3.1.0] Maximum call stack size exceeded (native stack depth)
2 participants