fix(breakpoints): races in useViewportWidth

[agriffis]

Summary

The main fix here is changing the initial state from null to
window.innerWidth, so that there isn't a useless transition from (for
example) useDown('md') = true to useDown('md') = false when the
application loads. Depending on the application, the rerendering penalty
can be heavy.

While we're here, notice that we can make a couple related changes:
First, since the initial state is set properly, we don't need a layout
effect, instead we can use a normal effect. Second, swap the order of
listener and setWidth to avoid the tiny race between them.

Test plan

This cropped up for me in storybook. I have a component that measures its
container to make image layout decisions. On the first load of the app, it would
work, but on hot reload, the image would be the wrong size.

The problem is that useDown('md') returns the wrong value the first time. It
was okay on initial app load because image loading slowed things down enough
that useDown('md') would return the right value before the measurement.
However on hot reload, when the image is already loaded, the measurement would
fire in the window of time between the bogus useDown and the right useDown.

Note that the measurement code also reruns on resize, but since there's no
resize occurring, it wouldn't know to remeasure.

Making this change fixed the problem for me.

[gregberge]

Hello @agriffis, your code is not compatible with SSR. Maybe the actual strategy is not the good one but it is compatible with SSR.

[agriffis]

I just changed the useState initialization to window?.innerWidth and I think that does the trick for SSR, right?

[gregberge]

I just changed the useState initialization to window?.innerWidth and I think that does the trick for SSR, right?

Nop window is undefined server-side, please do : typeof window === 'undefined' ? null : window.innerWidth. It should work.

[agriffis]

@gregberge I can make that change, but just to confirm that we are looking at the same thing...

The last commit I offered has window?.innerWidth which uses optional chaining (note the question mark). It compiles to the following in xstyled-core.cjs.js

  var _React$useState = React.useState((_window = window) == null ? void 0 : _window.innerWidth),
      width = _React$useState[0],
      setWidth = _React$useState[1];

which uses lenient equality, and undefined == null evaluates to true.

Please let me know if you would still like me to change it, thanks!

[agriffis]

@gregberge I know you're busy, but I'd love to get this sorted and merged when you get the chance to review my comment above.

[gregberge]

@gregberge I know you're busy, but I'd love to get this sorted and merged when you get the chance to review my comment above.

Hello, typeof window !== 'undefined' is not equivalent to window == null because on server-side, window is actually undefined and it will throw if you try to call it even to test it that is it undefined.

I was in holiday, but I am ready to merge after you made the change.

[agriffis]

Hello, typeof window !== 'undefined' is not equivalent to window == null because on server-side, window is actually undefined and it will throw if you try to call it even to test it that is it undefined.

Sheesh, of course you're right:

Welcome to Node.js v12.16.3.
Type ".help" for more information.
> window
Uncaught ReferenceError: window is not defined
> window = undefined
undefined
> window == null
true

[agriffis]

@gregberge Should be all set now

[agriffis]

@gregberge You may want to consider reverting this change because it breaks React SSR hydration.

When React hydrates, it assumes that the provided HTML matches the initial rendering produced by the client. If there is a mismatch, React won't know that it needs to update the DOM. The virtual DOM and real DOM are out of sync until a change to the virtual DOM triggers a sync.

For example:

Server: first (only) render, useViewPortWidth returns null, and the component renders foo

Client: first render, useViewPortWidth returns 800, which renders bar to the virtual DOM, but React trusts the hydrated SSR HTML. Even if the client forces a second render, React won't recognize that the virtual DOM and real DOM are out of sync, because the virtual DOM isn't changing.

There's another technique that I'm exploring that works like this:

const useMyHook = () => {
  // ...interesting code that probably depends on window.something...
  
  const [hydrated, setHydrated] = React.useState(false)
  React.useEffect(() => {
    setHydrated(true)
  }, [setHydrated])

  return hydrated ? value : null
}

This ensures a consistent initial return value between SSR and browser, so that the initial virtual DOM matches the real DOM during hydration.

[agriffis]

Also, I apologize for breaking React SSR hydration. I've learned a lot about it between this PR and now...

[gregberge]

Yeah I know, but there is no good answer here. I think it is preferable to break SSR rehydration in this case to avoid a blink if there is no SSR.

We could provide an option to choose the behavior.

[agriffis]

Well, full disclosure, I'm not using xstyled at the moment, because I made some deliberate choices to sacrifice functionality for the sake of fewer dependencies in my current project... However, I have an idea for this:

import {useEffect, useLayoutEffect, useState} from 'react'

const useIsomorphicLayoutEffect =
  typeof document === 'undefined' ? useEffect : useLayoutEffect

/**
 * Return viewport width, updating when the viewport changes. Pass defaultValue
 * (not undefined) to enable SSR hydration.
 */
export const useViewportWidth = defaultValue => {
  const [width, setWidth] = React.useState(
    typeof defaultValue !== 'undefined'
      ? defaultValue
      : typeof window === 'undefined'
      ? null
      : window.innerWidth,
  )

  useIsomorphicLayoutEffect(() => {
    const handleResize = () => setWidth(window.innerWidth)
    window.addEventListener('resize', handleResize)
    setWidth(window.innerWidth)
    return () => window.removeEventListener('resize', handleResize)
  }, [setWidth])

  return width
}

So there are two concepts here:

1. If you want SSR hydration consistency, pass defaultValue, otherwise you'll get the client-optimized behavior.

2. Use a layout effect to update width and hopefully avoid the blink in any case.