margelo / react-native-graph

📈 Beautiful, high-performance Graphs and Charts for React Native built with Skia
https://margelo.io
MIT License
2.08k stars 118 forks source link

Infinite loop with callbacks #6

Closed DDushkin closed 2 years ago

DDushkin commented 2 years ago

Hello, thanks for the package! Everything was good until I started rendering current point price and timestamp near the graph The code like this

const [amountTitle, setAmountTitle] = useState(assetBalanceFormatted);
const [timestamp, setTimestamp] = useState<DateTime>();
//...
<Center>
   <AssetIconText
      currency={assetName}
      subtitle={amountTitle ? amountTitle : assetBalanceFormatted}
   />
   <Text fontWeight="500" color="text.secondary">
      {timestamp
         ? timestamp.toFormat("d MMM yyyy HH:mm")
         : `${t("main.value")}: ${fiatBalanceFormatted}`}
   </Text>
<Center>
<LineGraph
     style={styles.graph}
     animated={true}
     color="#067dfb"
     dotColor="#ffffff"
     gradientColors={["#067dfb4d", "#37385f1a"]}
     lineThickness={1.5}
     points={graph.data.points}
     enablePanGesture={true}
     onGestureStart={() => hapticFeedback("impactLight")}
     onPointSelected={(p) => {
        console.log(p);
        const selectedPrice = formatAmount(
             p.value,
             fiatCurrency.code as CurrencyName,
             "fiat",
         );
         const timestamp = DateTime.fromSeconds(p.date);

         setAmountTitle(selectedPrice);
         setTimestamp(timestamp);
     }}
     onGestureEnd={() => {
         setAmountTitle("");
         setTimestamp(undefined);
     }}
/>

Creates infinite loop onPointSelected fires at the first render and then onGestureEnd fires next and so on

mrousavy commented 2 years ago

Oh yeah that's because it isn't memoized.

Try


const onPointSelected = useCallback(() => {
        console.log(p);
        const selectedPrice = formatAmount(
             p.value,
             fiatCurrency.code as CurrencyName,
             "fiat",
         );
         const timestamp = DateTime.fromSeconds(p.date);

         setAmountTitle(selectedPrice);
         setTimestamp(timestamp);
}, [........])

return <LineGraph YOURCODE />
DDushkin commented 2 years ago

Thanks!

apostgit commented 1 year ago

This problem still exists. onPointSelected works fine but when i set a function on onGestureEnd then it loops over and over again on both onPointSelected and onGestureEnd.

const on_point_selected = useCallback((point) => { setMovingPrice(point.value) console.log(point) }, []) const reset_price_title = useCallback(() => { setMovingPrice(token.price) console.log('s') }, [])

<LineGraph points={chartHistory} style={[styles.graph, { width: SIZE }]} animated={true} enablePanGesture={true} onGestureStart={() => Haptics.impactAsync(Haptics.ImpactFeedbackStyle.Light)} onPointSelected={(p) => on_point_selected(p)} onGestureEnd={() => reset_price_title()} indicatorPulsating={true} TopAxisLabel={() => <AxisLabels date={chartHistoryMax.date} value={chartHistoryMax.value.toFixed(2)} />} BottomAxisLabel={() => <AxisLabels date={chartHistoryMin?.date} value={chartHistoryMin?.value.toFixed(2)} left={true} />} verticalPadding={10} color={isLight ? token.theme_color : token.theme_color == '#000000' ? '#ffffff' : token.theme_color} lineThickness={2} />

chrispader commented 1 year ago

Can you use the memoized callbacks directly, instead of wrapping them with a arrow function in the LineGraph's props like:

<LineGraph
  ...
  // Memoize this too
  onGestureStart={() => Haptics.impactAsync(Haptics.ImpactFeedbackStyle.Light)}
  // Use callbacks like this:
  onPointSelected={on_point_selected}
  onGestureEnd={reset_price_title}
  ...
/>
apostgit commented 1 year ago

Works great!