My program goes into an infinite loop constantly calling useEffect() everytime I start the app. I have one state that I don't think is changing other than in the retrieveItemStatus() function so I'm confused on why its going into a loop like it is.
const App = () => {
var items;
const [itemStatuses, updateStatuses] = useState({});
const retrieveItemStatus = async () => {
var tempStatuses;
try {
const value = await AsyncStorage.getItem("@item_Statuses");
if (value !== null) {
tempStatuses = await JSON.parse(value);
//console.log("123456");
} else {
tempStatuses = await JSON.parse(
JSON.stringify(require("../default-statuses.json"))
);
}
updateStatuses(tempStatuses);
} catch (error) {}
};
retrieveItemStatus();
useEffect(() => {
const copyData = async () => {
const itemsCopy = [];
const coll = await collection(db, "Items");
const querySnapshots = await getDocs(coll);
const docsArr = querySnapshots.docs;
docsArr.map((doc) => {
var data = doc.data();
if (itemStatuses[data.name] === "locked") return;
itemsCopy.push(data);
});
items = itemsCopy;
//getItems([...itemsCopy]);
};
copyData();
}, [itemStatuses]);
return (
<View style={styles.container}>
<Text>temp.......</Text>
</View>
);
};
It has nothing to do with useEffect
. You're calling retrieveItemStatus
unconditionally every time your component function is called to render the componennt. retrieveItemStatus
calls updateStatuses
which changes state. You see your useEffect
callback get run repeatedly as a side-effect of that, because your useEffect
callback has itemStatuses
as a dependency.
I assume you only need the itemStatuses
to get fetched once. If so, put the call in a useEffect
callback with an empty dependency array:
useEffect(retrieveItemStatus, []);
Also, you have (note the ***
):
const App = () => {
var items // ***
// ...
useEffect(() => {
const copyData = async () => {
// ...
items = itemsCopy; // ***
// ...
};
copyData();
}, [itemStatuses]);
};
That won't work, by the time you assign to items
from the callback, anything you might have been trying to do with items
will already have just used undefined
(the value it gets when you don't give it one). If you need items
to be retained, either put it in state (if you use it for rendering) or in a ref (if you don't).
In a comment you said:
Ok so I put
retrieveItemStatus()
call insideuseEffect
and removed the dependency which fixed the looping. But now there is an issue whereitemStatuses
state doesn't get updated beforecopyData()
is called anditemStatuses
is needed.. so it doesn't do anything until I manually refresh/render the whole thing again.
If copyData
relies on the result from retrieveItemStatus
, then put the calls to each of them in the same useEffect
, not calling copyData
until you get the results from retrieveItemStatus
. Something along the lines of the below, though you'll need to tweak it of course as I don't have all the details (I've also made some other comments and changes in there I've flagged up):
// *** There's no need to recreate this function on every render, just
// have it return the information
const retrieveItemStatus = async () => {
try {
let tempStatuses; // *** Declare variables in the innermost scope you can
const value = await AsyncStorage.getItem("@item_Statuses");
if (value !== null) {
tempStatuses = await JSON.parse(value);
//console.log("123456");
} else {
// *** stringify + parse isn't a good way to copy an object,
// see your options at:
// https://stackoverflow.com/questions/122102/
tempStatuses = await JSON.parse(JSON.stringify(require("../default-statuses.json")));
}
return tempStatuses;
} catch (error) {
// *** Not even a `console.error` to tell you something went wrong?
}
};
// *** Similarly, just pass `itemStatuses` into this function
const copyData = async (itemStatuses) => {
const coll = await collection(db, "Items");
const querySnapshots = await getDocs(coll);
const docsArr = querySnapshots.docs;
// *** Your previous code was using `map` just as a loop,
// throwing away the array it creates. That's an anti-
// pattern, see my post here:
// https://thenewtoys.dev/blog/2021/04/17/misusing-map/
// Instead, let's just use a loop:
// (Alternatively, you could use `filter` to filter out
// the locked items, and then `map` to build `itemsCopy`,
// but that loops through twice rather than just once.)
const itemsCopy = []; // *** I moved this closer to where
// it's actually filled in
for (const doc of docsArr) {
const data = doc.data();
if (itemStatuses[data.name] !== "locked") {
itemsCopy.push(data);
}
}
//getItems([...itemsCopy]); // *** ?
return itemsCopy;
};
const App = () => {
// *** A new `items` is created on each render, you can't just
// assign to it. You have to make it a member of state (or use
// a ref if it's not used for rendering.)
const [items, setItems] = useState(null);
const [itemStatuses, setItemStatuses] = useState({});
// *** ^−−−−− the standard convention is `setXyz`.
// You don't have to follow convention, but it makes it easier
// for other people to read and maintain your code if you do.
useEffect(() => {
(async () => {
const newStatuses = await retrieveItemStatus();
const newItems = await copyData(newStatuses);
// *** Do you need `itemStatuses` to be in state at all? If it's
// only used for calling `copyData`, there's no need.
setItemStatuses(newStatuses);
setItems(newItems);
})().catch((error) => {
console.error(error);
});
}, []);
// *** You didn't show what you're using here, so it's hard to be
// sure what needs to be in state and what doesn't.
// Only put `items` or `itemStatuses` in state if you use them for
// rendering.
return (
<View style={styles.container}>
<Text>temp.......</Text>
</View>
);
};
Here are those links as links: