-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Remove useEffect from TimeElapsed #3741
Conversation
Your Render PR Server URL is https://social-app-pr-3741.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-con7vcmv3ddc7392kbtg. |
@@ -3,8 +3,6 @@ import React from 'react' | |||
import {useTickEveryMinute} from '#/state/shell' | |||
import {ago} from 'lib/strings/time' | |||
|
|||
// FIXME(dan): Figure out why the false positives |
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.
This is something unrelated about lint and types that's since been addressed.
|
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.
Nice one
React.useEffect(() => { | ||
const [prevTick, setPrevTick] = React.useState(tick) | ||
if (prevTick !== tick) { | ||
setPrevTick(tick) |
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.
Isn't that (bad setState in render)?
I mean this error:
Warning: Cannot update a component (
App
) while rendering a different component (Watever
). To locate the bad setState() call insideWatever
, follow the stack trace as described in https://reactjs.org/link/setstate-in-render
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.
That error is about updating a different component. Updating the same component is ok if it's following this specific pattern: https://react.dev/reference/react/useState#storing-information-from-previous-renders
It's not needed and forces an unnecessary second render pass.
See https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes.
Test Plan
Waited a minute, verified timestamps get updated.