react.dev icon indicating copy to clipboard operation
react.dev copied to clipboard

Confusing explanation in Challenge 2 of React Learn ("You Might Not Need an Effect")

Open SuperMo0 opened this issue 2 months ago • 3 comments

Summary

The explanation in Challenge 2 (Cache a calculation without Effects) of You Might Not Need an Effect is confusing and somewhat misleading. The challenge suggests that replacing the useEffect with useMemo (or extracting a NewTodo component) reduces the number of calls to getVisibleTodos(), but this isn’t accurate. The original code already avoids unnecessary recomputations.

Page

https://react.dev/learn/you-might-not-need-an-effect

Details

In the original implementation, getVisibleTodos() is only called when either todos or showActive change. Typing in the input only updates the text state, which isn’t part of the dependency array—so getVisibleTodos() is not called on every keystroke.

The proposed useMemo solution behaves the same way: it recalculates getVisibleTodos() when todos or showActive change, and not when text changes. As a result, it doesn’t reduce the number of calls compared to the original implementation. Console logs confirm this.

The second suggested approach (extracting NewTodo into a separate component) gives the impression that the initial code was calling getVisibleTodos() on every input change, but that’s incorrect. The original code was already optimized since text wasn’t part of the effect’s dependencies.

The documentation even says:

“This approach satisfies the requirements too. When you type into the input, only the text state variable updates. Since the text state variable is in the child NewTodo component, the parent TodoList component won’t get re-rendered. This is why getVisibleTodos() doesn’t get called when you type. (It would still be called if the TodoList re-renders for another reason.)”

However, this explanation is misleading because getVisibleTodos() wasn’t being called at every input in the first place.

In short:

The useMemo version doesn’t reduce calls to getVisibleTodos(), it's exactly the same.

The second version’s explanation implies a problem that didn’t exist in the original code.

SuperMo0 avatar Oct 26 '25 20:10 SuperMo0

. The challenge suggests that replacing the useEffect with useMemo (or extracting a NewTodo component) reduces the number of calls to getVisibleTodos(), but this isn’t accurate.

The wording might not be clear, but it's not saying this it will reduce the number of calls. The point is that the original code with an effect is using an effect in order to cache the result and not re-run the function - but you can do that without an effect and get the same result.

rickhanlonii avatar Nov 05 '25 21:11 rickhanlonii

ok I understand so maybe the wording should be changed to emphasize that the original code is using useEffect to avoid loading the data on every input, but your job is to achieve the exact same thing (avoid loading data on every input ) without using useEffect.

Also, useMemo wasn't mentioned yet, so perhaps Solution 2 could be Solution 1

SuperMo0 avatar Nov 06 '25 01:11 SuperMo0

Yeah, feel free to submit a PR to clarify. I'd keep the order the same though, there's really no learn docs for useMemo so I don't think it's a big deal that it's not introduced yet, and it would be confusing to move the code to another component and then move it back in with useMemo.

rickhanlonii avatar Nov 06 '25 02:11 rickhanlonii