Skip to content
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

useLayoutEffect warning during SSR #30

Closed
micah-redwood opened this issue Jun 26, 2023 · 11 comments
Closed

useLayoutEffect warning during SSR #30

micah-redwood opened this issue Jun 26, 2023 · 11 comments

Comments

@micah-redwood
Copy link
Contributor

Hey, first off, thanks for this really nifty hook, in particular the focus on performance and avoiding unnecessary re-renders!

I've been using it in a Vite project, and noticed I'm getting a warning during SSR b/c of useLayoutEffect

Warning: useLayoutEffect does nothing on the server, because its effect cannot be encoded into the server renderer's output format. This will lead to a mismatch between the initial, non-hydrated UI and the intended UI. To avoid this, useLayoutEffect should only be used in components that render exclusively on the client. See https://reactjs.org/link/uselayouteffect-ssr for common fixes.

This warning can be avoided by explicitly using useEffect instead of useLayoutEffect during SSR. Here's one really simple implementation:
https://usehooks-ts.com/react-hook/use-isomorphic-layout-effect

There's no change in behavior (both hooks are skipped during SSR). It's just a way of telling the framework that you know that this won't run on the server and are ok with that.

@micah-redwood
Copy link
Contributor Author

Added a PR in case that's helpful: #31

@micah-redwood
Copy link
Contributor Author

FWIW, when doing SSR in Vite, I also had to add a workaround for the default export not being used.

vite  # works fine during development
vite build  # TypeError: useViewportSizes is not a function

Fix:

// SSR workaround for default export not being used during SSR for some reason...
const useViewportSizesWorkaround = (
  'default' in useViewportSizes ? useViewportSizes.default : useViewportSizes
) as typeof useViewportSizes;

rob2d added a commit that referenced this issue Jun 26, 2023
potentially helps with second comment raised on Vite build in issue #30
@rob2d
Copy link
Owner

rob2d commented Jun 27, 2023

For the first issue/PR: thank you for this very elegant/simple solution! definite DX improvement to avoid logging on the server end and doesn't add a lot of overhead, so that has been merged w/no issues.

For the second issue: since I see your project uses TS, my first thought is it may have to do with parsing the type declarations in the d.ts since the default export has several sigs (as you saw, the build is in JS to prioritize build size for the nature of this util). Unfortunately, I haven't gotten a chance to use Vite builds with any apps yet.

I've fixed some potential issues there locally though and will bump to an alpha version/push up to NPM to confirm whether or not that helps -- and also to point to in interim; though I'm suspecting I may need to create a Vite build on my end to mess around with at some point this week though.

Anyway, thanks again for the helpful contribution!

Edit: pushed current changes onto npm @ use-viewport-sizes@0.7.0-alpha.0. Will revisit again this week to set up a Vite build.

@micah-redwood
Copy link
Contributor Author

Hey, thanks @rob2d ! This alpha version fixed the SSR warning, but I'm still having to do the workaround for importing this package w/ Vite during SSR (using vite-plugin-ssr for SSR and @vitejs/plugin-react-swc for React support)

All this to say, it's likely related to how Vite imports packages and not Typescript. The type inference appears to be working fine.

@micah-redwood
Copy link
Contributor Author

Hey @rob2d , I just realized that in the alpha version of this library, the window resize listener is no longer being added and I'm getting a value of 0 for the viewport width.

Not sure what's going on but when I reverted to 0.6.1 it's all working as expected. Here's my code:

// In React component
const isNarrowScreen = !useMinViewportWidth(500);
// use-min-viewport-width.ts
import { useEffect, useLayoutEffect } from 'react';
import useViewportSizes from 'use-viewport-sizes';

// SSR workaround for default export not being used during SSR for some reason...
const useViewportSizesWorkaround = (
  'default' in useViewportSizes ? useViewportSizes.default : useViewportSizes
) as typeof useViewportSizes;

export function useMinViewportWidth(minWidth: number) {
  // Only re-render when viewport width crosses this threshold
  const hasher = ({ vpW }: { vpW: number }): string => (vpW >= minWidth ? 'yes' : 'no');
  const [vpW, updateVpSizes] = useViewportSizesWorkaround({ hasher, dimension: 'w' });

  // Run once on client immediately after render (not during SSR)
  useIsomorphicLayoutEffect(() => {
    updateVpSizes();
  }, [minWidth]);

  return hasher({ vpW }) === 'yes';
}

// https://usehooks-ts.com/react-hook/use-isomorphic-layout-effect
export const useIsomorphicLayoutEffect =
  typeof window !== 'undefined' ? useLayoutEffect : useEffect;

@micah-redwood micah-redwood reopened this Jul 6, 2023
@rob2d
Copy link
Owner

rob2d commented Jul 7, 2023

Thanks for the update; things have gotten quite busy recently, so I sort of took it at face value that the alpha solved your issue after the merge/closed issue.

Just quick thoughts but: what I suspect is happening with regard to 0 width/height is in the SSR env, it may be beneficial for it to just not run at all, since viewport width/height and the window does not exist. So in hindsight: it might be best not to fall back to useEffect but instead simply not run when rendering on the server-side in the hook.

I'm not exactly sure of the heuristics with SSR + React specifically in terms of the hydration of state/persistence and hooks: but it may just be that the state of how hooks are binding are just persisting? So the callbacks are binding to window, but that cannot exist in SSR env where it is actually called since it's not yet in the browser/client.

I will install into a sample Next repo and hopefully update tonight. I would still like to explore Vite builds also, but not binding callbacks overall seems pretty important relatively speaking.

rob2d added a commit that referenced this issue Jul 7, 2023
should address issue #30.
also bumps to 0.7.0-alpha.1 for pushing to NPM
@rob2d
Copy link
Owner

rob2d commented Jul 7, 2023

@micah-redwood seems to thankfully be something simple we both just overlooked... 🙃

In the changes you proposed @ #31, useIsomorphicLayoutEffect is actually defined as a method containing a tertiary.

It should just be a variable that's defined above the hook since it is just either this or that method to run.

export const useIsomorphicLayoutEffect =
  typeof window !== 'undefined' ? useLayoutEffect : () => {};

For the thought above I also updated so that the effect is a no-op. It's probably minor but nice to know that no effect is added at all, since we don't want to call anything when there's no window technically.

Pushed on NPM to 0.7.0-alpha.1. May not address the Vite-specific workaround but hopefully things work now in SSR with no probs.

(thanks for following up on it up also... probably saves some confusion for people who wouldtry to run the alpha version using SSR)

@micah-redwood
Copy link
Contributor Author

micah-redwood commented Jul 7, 2023

🤦 oops! Thanks for catching that, and my apologies for not actually running this code locally before submitting the PR.

I was using constant ternary in my code and copied it to a regular function to match your code style, and forgot to add the return statement

function useIsomorphicLayoutEffect() {
    typeof window !== 'undefined' ? useLayoutEffect : useEffect; // Not returning anything
}

function useIsomorphicLayoutEffect() {
    return typeof window !== 'undefined' ? useLayoutEffect : useEffect; // Corrected
}

FWIW, I think it would be better to return useEffect for SSR since this is a React best practice (same number of hooks in component).

@rob2d
Copy link
Owner

rob2d commented Jul 8, 2023

Haha no worries happens to all of us!

I'm also aware of the practice: so an empty function is technically also a hook -- hence should work e.g. this would also be a valid hook according to Rules of Hooks:

function useNoop() {}

It just does not have an effect.

useEffect however will run even on SSR, and will not produce any meaningful output initially (but width/height are already defaulted to 0 in this env with the logic that exists, so it's somewhat redundant in SSR env to parse variables that aren't ready).

When it actually gets to the component for render, these are two different environments, and two different hooks. If we wrap it in a function, we are just creating an entirely new hook -- but this isn't necessary since we aren't yet in component scope when a hooks are read by a component. A hook is essentially at the end of the day function reference that gets binded to the component scope when running and must unwind in a predictable order each time (which this does, since server/client are separate envs).

In the original example provided, this is also defined without a containing method:

import { useEffect, useLayoutEffect } from 'react'

export const useIsomorphicLayoutEffect =
  typeof window !== 'undefined' ? useLayoutEffect : useEffect

So the additional overhead of a containing hook should not be necessary.

In any case, if you find any issues when running let me know. Otherwise, will promote this version next week at some point (and thank again!).

@micah-redwood
Copy link
Contributor Author

Sounds good, I'll try out use-viewport-sizes@0.7.0-alpha.1 and let you know if I run into any issues

@rob2d rob2d closed this as completed Jul 14, 2023
@rob2d
Copy link
Owner

rob2d commented Jul 14, 2023

@micah-redwood After a deep rabbit hole looking into it today, the Vite + SSR import seemed an inherent issue with how their builds are set up -- saw that another plugin of theirs had a similar sort of issue going on. I guess the size of the userbase on React might be obscuring things since from what I understand default exports aren't as common in Vue, so could always be good to post on their repo? From what I saw on their main site there is a specific discord for discussing Vite + SSR.

Considering that, your current approach might be the most-sane/practical for the time being. Sorry not to be able to add more insight/a better solution to that one.

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

No branches or pull requests

2 participants