Skip to content

Conversation

@zakingslayerv22
Copy link
Contributor

@zakingslayerv22 zakingslayerv22 commented Dec 18, 2025

Taylor Mclean’s article is good but it follows some patterns that should not be encouraged in react.

Because

This PR

The article contains React anti-patterns that were discouraged earlier in the react course.

  1. Using props in state - As taught in previous lessons, this is a React anti-pattern that duplicates data.
const GroupForm = ({ initialUsers = [] }) => {
//using props in state
const [users, setUsers] = useState(initialUsers);

//blocks of code
};
  1. Error in code – He defines a users prop in UserManagement, then in Form he references it as user.
const Form = () => {
   if (formOpen) {
       if (selectedUser) return <UserForm onSubmit={editUser} user={{}} />;
        //referenced it as user here
       else return <UserForm onSubmit={addUser} user={user[selectedUser]} />;
      }
   return null;
};
  1. Tests are brittle and inconsistent – The delete and edit tests are based on props while the add test is
    based on closure. I think a good test suite should be more consistent.

I think Peter Jacxsens’s article (although more simplistic) https://dev.to/peterlidee/mocking-react-components-jest-mocking-react-part-2-2l8j does a better job with mocking child components

Issue

Closes #XXXXX

Additional Information

Pull Request Requirements

  • I have thoroughly read and understand The Odin Project curriculum contributing guide
  • The title of this PR follows the location of change: brief description of change format, e.g. Intro to HTML and CSS lesson: Fix link text
  • The Because section summarizes the reason for this PR
  • The This PR section has a bullet point list describing the changes in this PR
  • If this PR addresses an open issue, it is linked in the Issue section
  • If any lesson files are included in this PR, they have been previewed with the Markdown preview tool to ensure it is formatted correctly
  • If any lesson files are included in this PR, they follow the Layout Style Guide

Taylor Mclean’s article is good but it follows some patterns that should not be encouraged in react.
@github-actions github-actions bot added the Content: React Involves the React course label Dec 18, 2025
@rlmoser99 rlmoser99 requested review from a team and wise-king-sullyman and removed request for a team December 20, 2025 23:06
Copy link
Contributor

@mao-sz mao-sz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion. There is already an open issue on this #26387 with similar suggestions.

I do agree the current article is a bit tricky to follow, especially with the lack of syntax highlighting. However, I feel like the proposed replacement is a little too simplistic, to the point where I can see learners taking what it says as absolute, like the idea that you must only ever unit test components and never actually render any child components. I do see people focusing way too much on the idea of "unit testing" or "integration testing" as categories rather than thinking about what they're testing in the moment and why they're testing it (which may involve more or less isolated tests depending on the situation).

I also don't like how it showcases tests that assert that child components' contents are not rendered, which is definitely not what I'd be writing tests for; those tests are superfluous and only really test that that specific test case successfully mocks a certain component. It says nothing about other tests nor anything about the user-facing behaviour of the site, which is the real point of us testing things.

I'm definitely up for having the first assignment article replaced but I'm not in any rush to do so, and not with the proposed replacement. Happy to review an alternative though, or if you're not up for this then you can close the PR (which is also fine).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Content: React Involves the React course

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mocking Callbacks And Components: hard to read article about mocking child components

2 participants