I am learning react and right now I have a react app with users and movies, where you can see info about movies whenever you are logged in. If you are logged in as an admin you get access to an adminpage where you can see a list of usercards and register another admin. This cardlist gets updated without needing to refresh.
Since the code that I wrote is not that clean, I wanted to incorporate custom hooks. The problem is that with the new custom hooks everything works fine except for the rendering. Whenever I delete a user or add a new admin, the cardlist does not get updated unless I refresh the page.
I now have a custom hook useUsers but I only use it for my input fields and toast notifcations.
I tried adding users to my useEffect in the hook but that didn't fix my problem.
useEffect(() => { refreshUserList(); }, [users]);
Here is my code.
function useUsers() {
const [users, setUsers] = useState([])
const [showModal, setShowModal] = useState(false)
const notifyUserDeleted = () => toast.success('User deleted!', {
position: "top-right",
autoClose: 3000,
hideProgressBar: false,
closeOnClick: true,
pauseOnHover: true,
draggable: true,
progress: undefined,
theme: "colored",
});
const [adminUsername, setAdminUsername] = useState("")
const [adminPassword, setAdminPassword] = useState("")
const [showAdminModal, setShowAdminModal] = useState(false)
const notifyAddAdminSuccess = () =>
toast.success('Admin registered!', {
position: "top-right",
autoClose: 3000,
hideProgressBar: false,
closeOnClick: true,
pauseOnHover: true,
draggable: true,
progress: undefined,
theme: "colored",
})
const refreshUserList = () => {
UserAPI.getUsers()
.then(res => {
setUsers(res.data.users);
})
.catch(err => {
console.error(err);
});
};
useEffect(() => {
refreshUserList();
}, []);
const createAdmin = (adminUsername, adminPassword) => {
const registeredAdmin = {
"username": adminUsername,
"password": adminPassword
};
UserAPI.createAdmin(registeredAdmin)
.then(() => {
setAdminUsername("");
setAdminPassword("");
notifyAddAdminSuccess();
Adminpage.refreshUserList();
})
.catch(err => {
console.log(err);
alert("Failed to register admin!");
});
};
const handleRegisterAdmin = (e) => {
e.preventDefault();
createAdmin(adminUsername, adminPassword);
setShowAdminModal(false);
};
const deleteUser = (id) => {
UserAPI.deleteUser(id)
.then(() => {
notifyUserDeleted()
refreshUserList()
})
.catch(error => {
console.error(error);
});
};
const handleDelete = (e) => {
e.preventDefault();
deleteUser(e.target.value)
setShowModal(false)
}
return {
users, setUsers, showModal, setShowModal, notifyUserDeleted, notifyAddAdminSuccess, showAdminModal, setShowAdminModal,
adminPassword, setAdminPassword, adminUsername, setAdminUsername, refreshUserList, handleRegisterAdmin, handleDelete
}
}
export default useUsers;
function Adminpage() {
const { users, refreshUserList } = useUsers();
return (
<div className="container" style={{ display: "flex" }}>
<UserCardList users={users} refreshUserList={refreshUserList} />
<InputAdmin refreshUserList={refreshUserList} />
</div>
);
}
export default Adminpage;
function UserCardList(props) {
return (
<div className="container">
<h4 style={{ margin: "3% 0 2% 0" }}>User list:</h4>
<Bootstrap.Row>
{
props.users.map(user =>
<UserCard key={user.id} users={user} refreshUserList={props.refreshUserList} />
)
}
</Bootstrap.Row>
</div>
)
}
export default UserCardList;
function UserCard(props) {
const { showModal,
setShowModal,
handleDelete
} = useUsers()
return (
<Bootstrap.Col className="col-lg-4 col-12">
<Bootstrap.Card className='mb-1' style={{ height: "98%", }}>
<Bootstrap.Card.Body>
<Bootstrap.Card.Text><b>User ID: </b>{props.users.id}</Bootstrap.Card.Text>
<Bootstrap.Card.Text><b>Username: </b>{props.users.username}</Bootstrap.Card.Text>
<Bootstrap.Button style={{ backgroundColor: "red", borderColor: "gray" }} onClick={() => setShowModal(true)}><RiDeleteBin5Fill style={{ backgroundColor: "red" }} /></Bootstrap.Button>
</Bootstrap.Card.Body>
</Bootstrap.Card>
<Bootstrap.Modal centered show={showModal} onHide={() => setShowModal(false)}>
<Bootstrap.Modal.Header closeButton>
<Bootstrap.Modal.Title>Confirm Delete</Bootstrap.Modal.Title>
</Bootstrap.Modal.Header>
<Bootstrap.Modal.Body>Are you sure you want to delete this user?</Bootstrap.Modal.Body>
<Bootstrap.Modal.Footer>
<Bootstrap.Button variant="secondary" onClick={() => setShowModal(false)}>
Cancel
</Bootstrap.Button>
<Bootstrap.Button variant="danger" value={props.users.id} onClick={handleDelete}>
Delete
</Bootstrap.Button>
</Bootstrap.Modal.Footer>
</Bootstrap.Modal>
</Bootstrap.Col>
)
}
export default UserCard;
CodePudding user response：
Issue
useEffect(() => { refreshUserList(); }, [users]);
Adding users to the useEffect hook will likely cause a render loop since refreshUserList ultimates updates the users state. Don't unconditionally update any of a hook's dependencies.
React hooks also don't share state. You've two components, Adminpage and UserCard, each using separate instances of a useUsers hook each with their own state. Mutating the state in one instance of useUsers doesn't effect any other instance of useUsers.
Solution
Move the state and logic from the useUsers to a singular React context provider and allow all instances of the useUsers hook to access the single context value.
Example:
export const UsersContext = React.createContext({
adminPassword: "",
adminUsername: "",
handleDelete: () => {},
handleRegisterAdmin: () => {},
notifyAddAdminSuccess: () => {},
notifyUserDeleted: () => {},
setAdminPassword: () => {},
setAdminUsername: () => {},
setShowAdminModal: () => {},
setShowModal: () => {},
setUsers: () => {},
showModal: () => {},
showAdminModal: false,
refreshUserList: () => {},
users: [],
});
export const useUsers = () => React.useContext(UsersContext);
const toastOptions = {
position: "top-right",
autoClose: 3000,
hideProgressBar: false,
closeOnClick: true,
pauseOnHover: true,
draggable: true,
progress: undefined,
theme: "colored",
};
const UsersProvider = ({ children }) => {
const [users, setUsers] = useState([]);
const [showModal, setShowModal] = useState(false);
const [adminUsername, setAdminUsername] = useState("");
const [adminPassword, setAdminPassword] = useState("");
const [showAdminModal, setShowAdminModal] = useState(false);
const notifyUserDeleted = () =>
toast.success('User deleted!', toastOptions);
const notifyAddAdminSuccess = () =>
toast.success('Admin registered!', toastOptions);
const refreshUserList = () => {
UserAPI.getUsers()
.then(res => {
setUsers(res.data.users);
})
.catch(err => {
console.error(err);
});
};
useEffect(() => {
refreshUserList();
}, []);
const createAdmin = (username, password) => {
const registeredAdmin = { username, password };
UserAPI.createAdmin(registeredAdmin)
.then(() => {
setAdminUsername("");
setAdminPassword("");
notifyAddAdminSuccess();
Adminpage.refreshUserList();
})
.catch(err => {
console.log(err);
alert("Failed to register admin!");
});
};
const handleRegisterAdmin = (e) => {
e.preventDefault();
createAdmin(adminUsername, adminPassword);
setShowAdminModal(false);
};
const deleteUser = (id) => {
UserAPI.deleteUser(id)
.then(() => {
notifyUserDeleted();
refreshUserList();
})
.catch(error => {
console.error(error);
});
};
const handleDelete = (e) => {
e.preventDefault();
deleteUser(e.target.value);
setShowModal(false);
}
const value = {
adminPassword,
adminUsername,
handleDelete,
handleRegisterAdmin,
notifyUserDeleted,
notifyAddAdminSuccess,
refreshUserList,
setUsers,
showModal,
setShowModal,
showAdminModal,
setShowAdminModal,
setAdminPassword,
setAdminUsername,
users,
};
return (
<UsersContext.Provider value={value}>
{children}
</UsersContext.Provider>
);
}
Wrap the app code with the UsersProvider component to provide the users state and callbacks.
<UsersProvider>
...
<Adminpage />
...
</UsersProvider>
Now all the components rendered in UsersProvider's sub-Reactree using the useUsers hook will access and reference the same state and callbacks.
CodePudding user response：
useEffect(() => { refreshUserList(); }, [users])
The 2nd parameter [users] tells useEffect when to fire. So you are actually refreshing the users when the users have changed, which then means never because the users change when they are refreshed.
CodePudding user response：
The trouble you face is very very common in React :) Each hook usage creates it's "own" isolated scope. Meaning that each useUsers() usage produces separate users collection (retrieved from your back-end inside useEffect) and exposes separate set of user-manipulation-specific methods (like handleDelete)
First of all, before answering initial question let's reveal very very dramatic trouble you'll probably face soon
As far as each "useUsers" execution has it's own scope, every time you use this hook, following code will get executed:
useEffect(() => {
refreshUserList(); // UserAPI.getUsers()
}, []);
Now imagine, you call this hook in Adminpage component - users get loaded from the back-end. When users are loaded - you render UserCardList with loaded users. And UserCardList renders UserCard component for each loaded user. And finally each UserCard component calls "useUsers" hook you've created and each of them calls code above (useEffect -> refreshUserList) so each of UserCard component loads users from your back-end one more time :(
You see the trouble, you load 20 users, and for each of these users you load all these users again. if you have 20 users - it would be 21 call on your back-end. If you have 100 users - it would be 101 call on the back-end, etc... The performance of the back-end will definitely degradade very very quickly
To prove this assumption please open network tab in your browser and just reload the page. You'll dfefinitely see endless sequence of load-users requests...
That was about the back-end, but the initial question with not working front-end when users are removed is still opened
The situation is following: you use "useUsers" in Adminpage component to render user list and you use "useUsers" in each UserCard component to call "handleDelete" whenever needed
Each of these components loads "users" independently and when user gets removed via handleDelete in UserCard component - yes, you remove this user "physically" from your storage via
UserAPI.deleteUser
But, on the front-end you execute "refreshUserList":
const deleteUser = (id) => {
UserAPI.deleteUser(id)
.then(() => {
notifyUserDeleted()
refreshUserList()
})
.catch(error => {
console.error(error);
});
};
only in scope of current UserCard component and it refreshes "users" collection only in scope of UserCard component
So parent Adminpage component does not have idea that one of users was removed and user list does not get updated
The most easy "working" solution here is not to call useUsers() hook multiple times and pass handleDelete from Adminpage component to each UserCard component directly (as input parameter):
function Adminpage() {
const { users, refreshUserList, handleDelete } = useUsers();
return (
<div className="container" style={{ display: "flex" }}>
<UserCardList handleDelete={handleDelete} users={users} refreshUserList={refreshUserList} />
<InputAdmin refreshUserList={refreshUserList} />
</div>
);
}
export default Adminpage;
function UserCardList(props) {
return (
<div className="container">
<h4 style={{ margin: "3% 0 2% 0" }}>User list:</h4>
<Bootstrap.Row>
{
props.users.map(user =>
<UserCard key={user.id} handleDelete ={props.handleDelete} users={user} refreshUserList={props.refreshUserList} />
)
}
</Bootstrap.Row>
</div>
)
}
export default UserCardList;
function UserCard(props) {
// do not call "useUsers" here !!! instead accept needed callbacks as input parameters
return (
<Bootstrap.Col className="col-lg-4 col-12">
......... this code remains the same ..........
This technique would fix all back-end / front-end issues you face with minimal effort, but this fix has some disadvantages anyway:
- you're able to use "useUsers" hook only once in component tree. And you have to remember this rule and always be careful with it
- you have to pass methods through all components in your component tree, like I did it with handleDelete above (Adminpage -> UserCardList -> UserCard). For your component tree it's not too complex, but for larger component hierarchies it could become "a hell". Just imagine that you do it for 5 callbacks through hierarchy of 10 components...
The first trouble (only single hook usage) could be fixed only if you'll connect your "useUsers" usages in some way. Probably, you could utilize some rxjs-like approach with subscribers/notifications inside useUsers, share same "users" collection between them, and whenever something happens inside one of "useUsers" - notify others, etc... Other possible solution here - utilize some application-wide storage, like REDUX and store users there. Anyway, that's definitely much complicated issue then one you already face and that's "better" to skip it if no urgency
To fix second issue (passing callback as input parameter through every component in hierarchy) React's useContext() hook could be handy https://beta.reactjs.org/reference/react/useContext