Dynamic list of posts implemented #1136
Conversation
src/components/StateContext.tsx
Outdated
| users, | ||
| setUsers, | ||
| selectedUser, | ||
| setSelectedUser, | ||
| error, | ||
| setError, | ||
| posts, | ||
| setPosts, | ||
| postsLoading, | ||
| setPostsLoading, | ||
| errorNotification, | ||
| setErrorNotification, | ||
| selectedPost, | ||
| setSelectedPost, | ||
| commentsFromPost, | ||
| setCommentsFromPost, | ||
| commentLoading, | ||
| setCommentLoading, | ||
| showCommentField, | ||
| setShowCommentField, | ||
| loadComments, | ||
| onDeleteComment, |
There was a problem hiding this comment.
consider creating handlers for state management functions and moving related logic into those handlers for better organization and readability instead of passing setState functions directly
src/components/UserSelector.tsx
Outdated
| useEffect(() => { | ||
| getUsers().then(setUsers); | ||
| }, [setUsers]); |
There was a problem hiding this comment.
including setUsers in the dependency array is redundant since it's stable and it's better for useEffect to run only once after initial render which is typically desired behavior for data fetching
|
I dont see this demo in preversion project in "README.md". Otherwise, I hope I did what you mean. I improved readability by creating a three files with state manegement. |
There was a problem hiding this comment.
overall you've made great improvements 🚀 but you're still passing useState setters through context which isn't recommended because they should always remain in component that creates them. consider creating handler functions for them or moving the entire logic that uses them into context
link for your demo you can always find in setting or in actions
src/components/PostContext.tsx
Outdated
| error, | ||
| setError, | ||
| errorNotification, | ||
| setErrorNotification, | ||
| posts, | ||
| setPosts, | ||
| postsLoading, | ||
| setPostsLoading, | ||
| selectedPost, | ||
| setSelectedPost, | ||
| showComments, | ||
| setShowComments, |
There was a problem hiding this comment.
the problem is that you're passing useState setters like setError and setPosts through context but they should stay in context because that's where they are created. To fix this you need to move all logic that uses them into context or create handler functions. Same is for all your contexts
src/components/FormContext.tsx
Outdated
There was a problem hiding this comment.
you don't need FormContext because you only use it inside NewCommentForm component
There was a problem hiding this comment.
we're going in circles so I think it's good enough but in future tasks try to plan what operations and functions context needs because you should't pass through setters of useState (setShowComments, setCommentsFromPost etc) for the same reason why you shouldn't pass them as props
https://robsza1122.github.io/react_dynamic-list-of-posts/