-
-
Notifications
You must be signed in to change notification settings - Fork 3
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
chore: avoid react linting errors (attempt #2) #85
chore: avoid react linting errors (attempt #2) #85
Conversation
✅ Deploy Preview for eslint-code-explorer ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
I tested that it does not cause issues when typing quickly into the editor (was the problem with #83). |
eca3d32
to
45eef42
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks.
() => | ||
debounce((value: string) => { | ||
onChange?.(value); | ||
}, 400), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const debouncedOnChange = useMemo(
() =>
debounce((value: string) => {
onChange?.(value);
}, 400),
[onChange],
);
How does useMemo
help in this case? We are returning a function, not a value. useCallback
is preferred when using a function. In this case, if we use useMemo
, it wouldn't have any impact, right? Is my understanding correct, or am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionally, it is equivalent, see also https://react.dev/reference/react/useCallback#how-is-usecallback-related-to-usememo
But for whatever reason, the react eslint plugin would warn when using useCallback
:
React Hook useCallback received a function whose dependencies are unknown. Pass an inline function instead. eslint(react-hooks/exhaustive-deps)
That error vanishes with useMemo
.
Prerequisites checklist
What is the purpose of this pull request?
Attempt #2 (after #83) to unblock #70.
By avoiding the 2 linting errors already present in the codebase.
What changes did you make? (Give an overview)
This time, I did not introduce any behavior changes.
I just replaced
useCallback
byuseMemo
because then the warningReact Hook useCallback received a function whose dependencies are unknown
vanishes.useEffect
for code path on mountuseCallback
byuseMemo
Related Issues
Is there anything you'd like reviewers to focus on?