0

I'm doing the notification page of my react native app. It has infinite scroll and "pull to refresh" options. Entering to the page it works, and it works also pulling to refresh. The problem occurs when I scroll down because it seems it calls server to fetch new notifications but it doesn't concatenate to the array.

import React, { useState, useEffect, useCallback, Component } from "react";
import {
  View,
  Text,
  FlatList,
  Button,
  Platform,
  ActivityIndicator,
  StyleSheet,
  ScrollView,
  RefreshControl,
  SafeAreaView,
} from "react-native";
import { useSelector, useDispatch } from "react-redux";
import i18n from "i18n-js";
import Colors from "../../constants/Colors";
import { getNotificationList } from "../../utils/NotificationsUtils";
import Card from "../../components/UI/Card";

const NotificationsScreen = (props) => {
  const [refreshing, setRefreshing] = useState(false);
  const [isLoading, setIsLoading] = useState(false);
  const [page, setPage] = useState(0);
  const [notifications, setNotifications] = useState([]);
  const [error, setError] = useState();

  const dispatch = useDispatch();

  const onRefresh = useCallback(async () => {
    setRefreshing(true);
    setNotifications([]);
    setPage(0);

    console.log("-- Refreshing --");

    getNotifications().then(() => {
      setRefreshing(false);
    });
  }, [dispatch, setRefreshing]);

  const fetchMoreNotifications = useCallback(async () => {
    const newPage = page + 7;
    setPage(newPage);
    console.log(
      "FETCH MORE from page " + newPage + " on array of " + notifications.length
    );

    getNotifications().then(() => {
      setIsLoading(false);
    });
  }, [dispatch, getNotifications]);

  const getNotifications = useCallback(async () => {
    setError(null);
    setIsLoading(true);
    try {
      console.log("Get from page " + page);
      // let fromRecord = (page - 1) * 7;
      const retrievedNotifications = await getNotificationList(
        page,
        7,
        true,
        false
      );
      console.log(
        "Setting " +
          retrievedNotifications.response.notifications.length +
          " new notifications on an already existing array of " +
          notifications.length +
          " elements"
      );

      let updatedNews = notifications.concat(
        retrievedNotifications &&
          retrievedNotifications.response &&
          retrievedNotifications.response.notifications
      );
      setNotifications(updatedNews);
    } catch (err) {
      setError(err.message);
    }
    setIsLoading(false);
  }, [dispatch, setIsLoading, setNotifications, setError]);

  useEffect(() => {
    setIsLoading(true);
    getNotifications(page).then(() => {
      setIsLoading(false);
    });
  }, [dispatch, getNotifications]);

  return (
    <View>
      {error ? (
        <View style={styles.centered}>
          <Text>Error</Text>
        </View>
      ) : refreshing ? (
        <View style={styles.centered}>
          <ActivityIndicator size="large" color={Colors.primary} />
        </View>
      ) : !notifications || !notifications.length ? (
        <View style={styles.centered}>
          <Text>No data found</Text>
        </View>
      ) : (
        <FlatList
          refreshControl={
            <RefreshControl refreshing={refreshing} onRefresh={onRefresh} />
          }
          data={notifications}
          keyExtractor={(notification) => notification.notificationQueueId}
          onEndReached={fetchMoreNotifications}
          onEndReachedThreshold={0.5}
          initialNumToRender={4}
          renderItem={(itemData) => (
            <View
              style={{
                marginTop: 10,
                height: 150,
                width: "100%",
              }}
            >
              <Card style={{ height: 150, backgroundColor: "white" }}>
                <Text style={{ fontSize: 16, color: Colors.black }}>
                  {itemData.item.text}
                </Text>
              </Card>
            </View>
          )}
        />
      )}
    </View>
  );
};

const styles = StyleSheet.create({
  centered: {
    flex: 1,
    justifyContent: "center",
    alignItems: "center",
  },
});

export default NotificationsScreen;

If I scroll to end it triggers 'fetchMoreNotifications' function and I get this in the console:

FETCH MORE from page 7 on an array of 0
Get from page 0
Setting 7 new notifications on an already existing array of 0 elements
FETCH MORE from page 7 on an array of 0
Get from page 0
Setting 7 new notifications on an already existing array of 0 elements
FETCH MORE from page 7 on an array of 0
Get from page 0
Setting 7 new notifications on an already existing array of 0 elements
...and so on

As you can see it says 'existing array of 0 elements' even if previously I saved notifications. Maybe it has some issue with useCallback's dependency?

panagulis72
  • 2,129
  • 6
  • 31
  • 71
  • Where is your reducer where you are trying to concatenate your array? – Waheed Akhtar Jun 02 '20 at 20:11
  • I have seen you are getting your notifications here with `getNotificationList ` are your sure you are getting data on the second attempt? – Waheed Akhtar Jun 02 '20 at 20:14
  • Hi Waheed, you gave me a good suggestion. Indeed the getNotificationList works fine, but it always retrieves the set 0-7 of notifications, so the FlatList doesn't show new notifications (because they have the same id). Debugging again the code, I saw that there is a weird behaviour (and I guess this is the real problem) with the page. Indeed, when the "fetchMoreNotifications" is triggered the "newPage" value is the correct one (newPage + 7), but inside "getNotifications" the "page" value is 0, how could it be? – panagulis72 Jun 03 '20 at 05:41
  • Hi panagulis, this issue requires debugging, I tried to create a snack here https://snack.expo.io/ but I don't have your other files, Can you make a snack here with the issue you have and share the link, I will fix it. – Waheed Akhtar Jun 03 '20 at 09:21
  • @WaheedAkhtar thank you for your support, it would be difficult to reproduce but I think the error is here: const fetchMoreNotifications = useCallback(async () => { let newPaginationValue = pagination.page + 7; console.log("Try to setting new pagination: " + newPaginationValue); await setPagination({ page: newPaginationValue }); console.log("setted: " + pagination.page); }, [dispatch, setPagination]); When I try to scroll down, the first console log is: Try to setting new pagination: 7, and it is right because I want now results from 7, but then "setted: 0" – panagulis72 Jun 04 '20 at 20:39
  • As you can see I tried to transform "[page, setPage]..." into an object: const [pagination, setPagination] = useState({ page: 0 }); – panagulis72 Jun 04 '20 at 20:39
  • It seems the state doesn't update – panagulis72 Jun 04 '20 at 20:40
  • Could you make a snack of it on expo or share me a reproducible demo? – Waheed Akhtar Jun 04 '20 at 21:46

2 Answers2

2

Issue :

There are 2 main issues, one with page and second with notifications, due to useCallback and dependencies, useCallback function will always point to the old values which are not in dependencies until one of the dependencies for updated.


1) The solution to page issue :

Pass newPage as param to getNotifications, due to async behavior of setPage it will not get updated directly

And on the second time, to get the updated value of page you can pass page as a dependency.

2) The solution to the notification issue :

Update the notification directly from its prev state value with setState(prevState => newState).


Solution :

  const fetchMoreNotifications = useCallback(async () => {
    const newPage = page + 7;
    setPage(newPage);
    console.log(
      "FETCH MORE from page " + newPage + " on array of " + notifications.length
    );
    getNotifications(newPage).then(() => { // <---- Pass as param
      setIsLoading(false);
    });
  }, [page]); // <---- Add page as dependency 

  const getNotifications = useCallback(
    async page => { // <---- Get page as a param
      setError(null);
      setIsLoading(true);
      try {
        console.log("Get from page " + page);
        // let fromRecord = (page - 1) * 7;
      const retrievedNotifications = await getNotificationList(
        page,
        7,
        true,
        false
      );

      setNotifications(prevNotification => prevNotification.concat(
        retrievedNotifications &&
          retrievedNotifications.response &&
          retrievedNotifications.response.notifications
      )); // <---- Setting up state directly from previous value, instead of getting it from clone version of use callback
      } catch (err) {
        console.log(err);
        setError(err.message);
      }
      setIsLoading(false);
    },
    [setIsLoading, setNotifications, setError]
  );

WORKING DEMO :

Check the console log for updated page value and notification will be rendered on Html it self

Edit nostalgic-sound-r07x2

NOTE : Removed some of your code just to improve code readability and debug the issue

Vivek Doshi
  • 56,649
  • 12
  • 110
  • 122
  • thank you very much, not only now it works but also I understood the mistake (and this was what I was really looking for). Just a note, I had to replace empty array with [dispatch] on dependencies of useEffect(()) to avoid this issue: open the page --> useEffect --> getNotifications --> fetchMoreNotifications --> getNotifications. So with [dispatch] dependency the "fetchMoreNotifications" is not triggered suddently when I open the page – panagulis72 Jun 06 '20 at 09:08
  • Glad to know, have a nice day. :) – Vivek Doshi Jun 06 '20 at 09:16
1

The problem is really simple. The getNotifications function is created using useCallback and hasn't used notifications as a dependency. Now when notifications updates, the getNotications function is still referring to the old notifications values due to closure.

Also note that you call getNotifications on fetchMoreNotifications immediately after setting page state but page state too is bound by closure and will not update in the same re-render

The solution here is to use the function approach to setNotifications and use useEffect to trigge4r getNotification on page change

const fetchMoreNotifications = useCallback(async () => {
    const newPage = page + 7;
    setPage(newPage);
  }, [dispatch, getNotifications]);

  useEffect(() => {
    setIsLoading(true);
    getNotifications(page).then(() => {
      setIsLoading(false);
    });
  }, [dispatch, page, getNotifications]);

const getNotifications = useCallback(async () => {
    setError(null);
    setIsLoading(true);
    try {
      console.log("Get from page " + page);
      // let fromRecord = (page - 1) * 7;
      const retrievedNotifications = await getNotificationList(
        page,
        7,
        true,
        false
      );

      setNotifications(prevNotification => prevNotification.concat(
        retrievedNotifications &&
          retrievedNotifications.response &&
          retrievedNotifications.response.notifications
      ));
    } catch (err) {
      setError(err.message);
    }
    setIsLoading(false);
  }, [dispatch, setIsLoading, setNotifications, setError]);
Shubham Khatri
  • 270,417
  • 55
  • 406
  • 400
  • Hi Shubham, thanks for the suggestion, I try so! Have you read the comments above? I thought the problem was in setting up the new page, because it seemed as if it didn't update the new value of the new pagination – panagulis72 Jun 05 '20 at 07:28
  • Ohh, yes sorry about that didn't read them carefully. There is another problem in how you call the getNotifications function on `fetchMoreNotifications` I am updating my answer for it – Shubham Khatri Jun 05 '20 at 07:47
  • Please check the update and let me know if it doesn't work – Shubham Khatri Jun 05 '20 at 15:24
  • Unfortunatly no, it doesn't work, but thanks anyway – panagulis72 Jun 06 '20 at 09:02