Skip to content

Fix useLayoutEffect throwing warnings when using SSR #3389

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

latekvo
Copy link
Contributor

@latekvo latekvo commented Feb 6, 2025

Warning

This PR may cause a regression, more info here.

Test SSR + react-freeze before pushing any of these changes!

Description

Using useLayoutEffect in combination with SSR results in a warning.
This PR adds a useSafeLayoutEffect, which acts as useLayoutEffect in regular apps, and defaults to useEffect when using SSR.

fixes #3375

Test plan

@@ -100,3 +102,6 @@ export function deepEqual(obj1: any, obj2: any) {
}

export const INT32_MAX = 2 ** 31 - 1;

export const useSafeLayoutEffect =
typeof window !== 'undefined' ? useLayoutEffect : useEffect;
Copy link
Contributor

@satya164 satya164 Mar 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

window isn't a good way to detect a browser environment. typeof document !== 'undefined' would be better to detect browsers. but it'll return false in react native environment, so something like following may be better:

Platform.OS !== 'web' || typeof document !== 'undefined' ? useLayoutEffect : useEffect;

Copy link
Contributor Author

@latekvo latekvo Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @satya164, thank you for Your feedback.

We've found this implementation, which seems the most comprehensive out of the ones we've seen.

Despite this, we decided to halt any changes in this PR for now, as using useIsomorphicEffect would likely only hide the SSR warning, while not preventing the issue that caused us to use the useLayoutEffect in the first place.

You can find the full explanation for our decision here.

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

Successfully merging this pull request may close these issues.

useLayoutEffect error on the terminal
2 participants