Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recommended pattern MyComp = observer(function MyComp() {...}) leads to subtle bug #3836

Open
Venryx opened this issue Feb 22, 2024 · 0 comments

Comments

@Venryx
Copy link
Contributor

Venryx commented Feb 22, 2024

Relevant section in the documentation:

The following approaches can be used to fix this:
- use `function` with a name instead of an arrow function. `mobx-react` infers component name from the function name:
```javascript
export const MyComponent = observer(function MyComponent(props) {
return <div>hi</div>
})
```

While the code snippet above works fine for this simple case, it leads to a very confusing and subtle bug, as seen below:

export const TodoItem = observer(function TodoItem(props) {
    const {item} = props;
    return (
        <div>
            <div>{item.text}</div>
            {item.children.map(childItem=>{
                return <TodoItem key={childItem.id} item={childItem}/>;
            })}
        </div>
    );
})

Anyone spot the problem?

The problem is that because the inner TodoItem function is exactly the same name as the outer TodoItem variable, when you reference TodoItem from within the function itself, JavaScript thinks you are referring to that inner function, rather than the outer variable.

Result: The child TodoItem components silently lack any mobx reactivity, due to their not having the observer() wrapper applied to them.

Suggestion: Change the example code to have the inner function named MyComponent_ or MyComponent_impl instead, and add a short note to that section explaining why this was done (so others don't remove that "unnecessary underscore" when writing their actual component, and hit this very subtle bug).


I raise this issue because I had this exact problem, and wasted about half an hour perplexed at why the children components were just inexplicably not responding to any mobx changes, and yet when I made an exact copy of the component and used that for the children, things magically worked again.

Turns out, my "exact copy except I changed the name" was itself how it was fixed, ie. by merely changing the component's name, I somehow fixed the issue. Then the reason that worked finally hit me.

(This issue might seem obvious to people viewing the code above with fresh eyes, but when you're dealing with an actual component with lots of contents, there are other things you're looking at that can distract you from finding the real problem, ie. variable name-shadowing.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant