-
Notifications
You must be signed in to change notification settings - Fork 14
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: resolve react-hooks/exhaustive-deps warnings #1991
Changes from all commits
67c0cd3
38ebfea
0f273b4
1d32df2
7a550ce
898ef6d
b564a1c
0a511ef
dad2618
2ea8ce8
a9b1c91
5be13ae
6065471
a250984
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,7 +65,7 @@ export const useNodes = ( | |
const [initialized, setInitializedState] = useState<boolean>(false) | ||
const initialNodes = useMemo(() => { | ||
return getDynamicNodes(rootNode.descendants()) | ||
}, []) | ||
}, [rootNode]) | ||
denieler marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const dynamicNodes = useMemo(() => { | ||
const latestNodes = rootNode.descendants() | ||
|
||
|
@@ -85,9 +85,10 @@ export const useNodes = ( | |
}) | ||
}, [rootNode, initialNodes]) | ||
|
||
// we have to render nodes twice: first for the initial showing data, and the second one — with the correct positions. | ||
const nodes = useMemo<DynamicPointNode[]>(() => { | ||
return updateNodesYPosition(dynamicNodes) | ||
// we have to render nodes twice: first for the initial showing data, and the second one — with the correct positions. | ||
// eslint-disable-next-line react-hooks/exhaustive-deps | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because of the comment above: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we maybe fix it properly by doing a hook that would run a function on the second render? |
||
}, [dynamicNodes, initialized]) | ||
|
||
useLayoutEffect(() => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -184,6 +184,7 @@ export const useScreens = <T = unknown>() => { | |
|
||
return defaultValue | ||
}, | ||
// eslint-disable-next-line react-hooks/exhaustive-deps | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because otherwise, eslint adds There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was a bug with eslint. I updated the dependency and it seems ok now |
||
[screenKey] | ||
) | ||
} | ||
|
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 was interesting 😁