artsy / fresnel

An SSR compatible approach to CSS media query based responsive layouts for React.
https://artsy.github.io/blog/2019/05/24/server-rendering-responsively
Other
1.26k stars 65 forks source link

Breaking change with latest React experimental builds #260

Closed gurkerl83 closed 1 year ago

gurkerl83 commented 2 years ago

Hi, I have used this library for a while now, also adopting React 18 using their experimental builds. The latest refactorings in Reacts hydration handling approach (including throwing errors when the content of a component does not match anymore) indicates a breaking change towards fresnels idea of sending variants of an element (desktop/mobile) prepared by a server back to the client.

Testing the latest experimental versions of React, I have identified the last version where fresnel is still compatible; a newer nightly version from the next day up to the latest experimental version from today is incompatible with fresnel.

Latest compatible version: 18.0.0-rc.0-next-fa816be7f-20220128 First incompatible version: 18.0.0-rc.0-next-3a4462129-20220201 Most recent incompatible version: 18.0.0-rc.0-next-79ed5e18f-20220217

Looking at the source code and commit messages where this shift all started seems they throw an error when a mismatch occurs. It seems they are not just warning the user anymore but engaging in resolving. This results in a client-side render, and all server-side output is lost.

Communication with the React team is essential to keep fresnels approach working with the upcoming React version. It is still an excellent strategy to prepare multiple versions of a component at the server-side and decide on the client which one to pick.

See the commit message for the new approach React follows https://github.com/facebook/react/commit/848e802d203e531daf2b9b0edb281a1eb6c5415d

Recent commits made towards their new hydration approach https://github.com/facebook/react/commits/51c8411d9dab33290b794fe716b9514e7db1fb68/packages/react-reconciler/src/ReactFiberWorkLoop.new.js

Thx!

damassi commented 2 years ago

Thank you so much for this report @gurkerl83! We'll look into it, and if you have any suggestions for implementing a fix we will happily accept PRs πŸ™

gurkerl83 commented 2 years ago

I did some digging into the recent changes in React and may have been able to identify the problem.

The initial report from above describes an error thrown when server-side generated components no longer match those on the client-side. This change of application behavior was introduced in the following commit.

https://github.com/facebook/react/commit/3f5ff16c1a743bce3f3d772dc5ac63c5fd2dac0a

In the same commit, further changes are made, with at least the following leading to another problem (assuming error throwing is disabled).

https://github.com/facebook/react/blob/3f5ff16c1a743bce3f3d772dc5ac63c5fd2dac0a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js#L563

if (nextInstance) {
  if (shouldClientRenderOnMismatch(fiber)) {
   warnIfUnhydratedTailNodes(fiber);
   throwOnHydrationMismatchIfConcurrentMode(fiber); // => (*2)
  }
  else { // => (*1)
   while (nextInstance) {
     deleteHydratableInstance(fiber, nextInstance);
     nextInstance = getNextHydratableSibling(nextInstance);
   }
  }
}

Local tests show that the condition statement "if/else" block is wrong; the delete operation must always be executed.

// *1 The delete operation needs to be called anyway; otherwise, DOM and React get out of sync, meaning phantom DOM entries (duplicated DOM Elements) get generated when re-rendering occurs. Those elements do not have a corresponding react component in the dev tools.

// *2 Throwing errors have to be optional, not mandatory, options to think about

I will open a related issue in the React project and leave a link to it.

Thx!

damassi commented 2 years ago

Thank you @gurkerl83 for the digging πŸ™

KrustyC commented 2 years ago

is there any update on this, or anything I could help with? I am facing the same issue after an update to React 18 (I am using NextJs)

gurkerl83 commented 2 years ago

Hi, unfortunately I have sad news, the final version of React 18 no longer supports the approach used in Fresnel (prepare multiple variants of a component on the server, send both to the client and select the respective variant on the client). In React, this very selection performed at the client is called DOM patching.

In my opinion, the way this so-called DOM patching is now implemented in React has flaws; actual errors are thrown across the library. I've looked at the source code of the affected places and everything I know about throwing and catching errors unfortunately doesn't apply here. Imagine if there were multiple, say 10 component pairs; it seems that error handling is repeatedly delegated to the root node of the application.

Attempts to catch the errors locally in suspense boundaries have also failed.

On the Fresnel project page it should be pointed out that the library is currently not compatible with React 18.

One of the core contributors has given a hint how the approach used by Fresnel could be implemented after all see here.

https://github.com/facebook/react/issues/23381#issuecomment-1079494600

Maybe Fresnel can be adjusted to this, although I don't understand that exactly.

The "supported" way to render different things was to do two-pass rendering by setting state in an effect. So what you could do is serialize which layout was rendered by the server together with the HTML. Then, on the client, make the initial render use that layout (instead of reading it from the client device). And then immediately do a re-render with the actual true layout. This would do more rendering work, but would guarantee a consistent result that doesn't suffer from hydration bugs.

damassi commented 2 years ago

Will update the README with a note next week after I get back from OOO, thanks again for the research πŸ‘

alloy commented 2 years ago

@damassi Would be good to at least try if what Dan mentions here actually works. As he mentions, though, it’s not intended to be used that way.

gurkerl83 commented 2 years ago

First attempts to solve the problem draw a smile on my face.

I follow the second option suggested by Dan.

patch up the HTML tree to prune the invisible branches before starting hydration.

Necessary steps

  1. wrapping trees in (which defers their hydration)
  2. update state to delete the extra variants before they have a chance to hydrate.

Where the adjustments are necessary?

  1. a change in fresnel is required
  2. can be implemented in the application code by the user, but offloading to fresnel seems reasonable.

Then effects from those branches shouldn't fire.

In fact, only the component selected at the client gerate fires; this seems great so far.

I test this both with the variants supported by Fresnel

The error messages are gone, but other aspects need to be considered to see if this can really be the solution concept.

I'll keep you updated....

Thx!

damassi commented 2 years ago

Awesome @gurkerl83! Thank you for digging into this πŸ™

gurkerl83 commented 2 years ago

Using this approach in render props, Fresnel is compatible with React 18, requiring customization only in the application code.

The other option of a dedicated div has yet to be elaborated on, but I suspect that the approach shown can work there.

A better option is, of course, to hide this coupling of second suspense barriers from the application code, but that can be done later. At this point, it was important whether the approach shown by Dan could work practically.

import { Variant } from '@mui/material/styles/createTypography';
import { FC, Suspense, useEffect, useState } from 'react';

import { StyledTypography } from './InteractiveHead.svc';

interface InteractiveHeadProps {
  variant: Variant;
  className?: string;
  renderChildren?: boolean;
}

export const InteractiveHeadWrapper: FC<InteractiveHeadProps> = ({
  variant,
  className,
  renderChildren,
  children
}) => {
  const [renderCount, setRenderCount] = useState(0);

  useEffect(() => {
    if (renderChildren === false) {
      setRenderCount(1);
    }
  }, [renderChildren]);

  /**
   * 1. The first suspense barrier is needed to prevent the child node of the
   * component pair from rendering that currently does not match the breakpoint.
   *
   * Note:
   * As of version 18, React no longer performs DOM patching internally.
   * The application code must now enforce this.
   *
   * 1.1 An effect is necessary to exclude the invalid child nodes from the server-generated
   * component pair but not the valid ones. This is essential to always pass the correct
   * child node in the first render pass; otherwise, you will experience a flicker of
   * the component and the surrounding components.
   *
   * 1.2 The following condition is important to cancel the invalid children; without React complaining.
   *
   * !renderChildren && renderCount === 1
   *
   * The invalid child node is excluded by running the effect the first time.
   *
   * Note:
   * Since this example is the render-prop variant, in which the element
   * itself takes the class name, the effect is not a problem here.
   * A problem is when this component defines other effects.
   *
   * 2. The enclosed second suspense barrier ensures that only the
   * effects of the selected and valid child nodes are fired.
   */

  return (
    <StyledTypography variant={variant} className={className}>
      <Suspense fallback={null}>
        {!renderChildren && renderCount === 1 ? null : (
          <Suspense fallback={null}>{children}</Suspense>
        )}
      </Suspense>
    </StyledTypography>
  );
};

Thx!

KarthikeyanRanasthala commented 2 years ago

Any update on this? :(

gurkerl83 commented 2 years ago

Any update on this? :(

Unfortunately not :(

damassi commented 2 years ago

Unfortunately things are very busy over here ATM. Will happily review/accept PRs for this approach, and we can discuss more over there πŸ‘

JensMou commented 2 years ago

Any updates on this? Or an alternative, that is not pure css? πŸ™

gurkerl83 commented 2 years ago

Hopefully, the following is not another false positive; it seems two things have to happen to resolve the compatibility issue.

A: A minor modification in fresnels sources

The affected component is ResponsiveProvider in the file DynamicResponsive.

mediaQueryStatusChangedCallback = () => {
   const mediaQueryMatches = this.checkMatchers(
      this.state.mediaQueryMatchers!
   )

   // wrap setState in startTransition
   startTransition(() => {
      this.setState({
         mediaQueryMatches
      })
   })
}

B: A Different approach to how the Media component gets consumed in the userland is necessary

B.1: Add the following Suspender component

import { FC } from 'react';

interface StorageRef {
  promise: Promise<void>;
  resolve: (value: void | PromiseLike<void>) => void;
  thrown: boolean;
  resolved: boolean;
}

const promiseCache = new Map<string, StorageRef>();

interface SuspenderProps {
  id: string;
  freeze: boolean;
}

export const Suspender: FC<SuspenderProps> = ({
  id,
  freeze,
  children
}) => {
  if (!promiseCache.has(id)) {
    let resolve;
    const promise = new Promise<void>(resolvePromise => {
      resolve = resolvePromise;
    });

    const promiseItem = {
      promise,
      resolve,
      thrown: false,
      resolved: false
    };

    promiseCache.set(id, promiseItem);
  }

  if (freeze && promiseCache.has(id)) {
    const promiseItem = promiseCache.get(id);

    // console.log('throw before:', id);

    if (promiseItem && promiseItem.thrown === false) {
      const promiseItemClone = {
        ...promiseItem,
        thrown: true,
        resolved: false // reset resolved
      };

      promiseCache.set(id, promiseItemClone);

      // console.log('throw:', id);
      throw promiseItem.promise;
    }
  } else if (!freeze && promiseCache.has(id)) {
    const promiseItem = promiseCache.get(id);
    if (
      promiseItem &&
      promiseItem.thrown === true &&
      promiseItem.resolved === false
    ) {
      // console.log('resolve:', id);
      promiseItem.resolve();

      const promiseItemClone = {
        ...promiseItem,
        resolved: true,
        thrown: false // reset throw
      };
      promiseCache.set(id, promiseItemClone);
      return children;
    }
  }

  return children;
};

B.2: Use the Suspender component within Suspense as follows here for a mobile drawer

export const MobileDrawer: FC<DrawerProps> = ({
  renderChildren,
  className,
  children
}) => {
  return (
    <Suspense fallback={null}>
      <Suspender
        id='mobile-drawer'
        freeze={!renderChildren}
      >
        <SwipeableDrawer
          className={className}
        >
          {children}
        </SwipeableDrawer>
      </Suspender>
    </Suspense>
  );
};

B.3: Use the Suspender component within Suspense as follows here for a desktop drawer

export const DesktopDrawer: FC<DrawerProps> = ({
  renderChildren,
  className,
  children
}) => {
  return (
    <Suspense fallback={null}>
      <Suspender
        id='desktop-drawer'
        freeze={!renderChildren}
      >
        <Drawer
          className={className}
        >
          {children}
        </Drawer>
      </Suspender>
    </Suspense>
  );
};

B.4: Use the components in Media using the render props variant

<>
   <Media lessThan='md'>
      {(className, renderChildren) => {
         return (
            <MobileDrawer
              className={className}
              renderChildren={renderChildren}
            >
               {mobile content}
            </MobileDrawer>
         );
         }}
      </Media>
      <Media greaterThanOrEqual='md'>
         {(className, renderChildren) => {
            return (
               <DesktopDrawer
                  className={className}
                  renderChildren={renderChildren}
               >
               {desktop content}
            </DesktopDrawer>
         );
         }}
      </Media>
</>

I can provide a merge request in the coming days.

A question remains. Should the Suspense boundary, including the Suspender component, be integrated into a library or live in the userland? Thx!

damassi commented 2 years ago

That's a great question. Would you mind elaborating a bit on what the Suspender component does in this case?

And in your example B.4, does that component live within <Media>? Ideally we don't need to change the API requirements and if an interim <Suspense> boundary is needed in order to catch things I don't think it's that much of a problem to include it within the library. The problem however is: suspended content further down the tree could get mistakenly caught by unaware devs.

If I'm understanding correctly:

<Suspense fallback={foo}>
  <Media><SomeComponentThatSuspends /></Bar>
</Suspense>

The upper suspense boundary would never fire and be unexpectedly claimed by the buried suspense boundary in <Media>. That could lead to pretty serious confusion.

gurkerl83 commented 2 years ago

@damassi React 18 does not remove unmatched elements when server and client components differ during the first hydration attempt anymore. This was the default behavior before React 18; unfortunately, they are more strict now.

The general idea is to suspend the component that does not match the current breakpoint condition before the initial hydration attempt gets executed; the Suspender component does that.

The example provided is for the render props variant fresnel supports. I have not looked at the component variant yet, but I guess the Suspense / Suspender combo needs to be integrated directly into fresnel.

One important thing is that only the component fire's lifecycles meet the current breakpoint condition.

And in your example B.4, does that component live within

It does not; see the respective desktop and mobile drawer components.

 <Suspense fallback={null}>
      <Suspender
        id='desktop-drawer'
        freeze={!renderChildren}
      >

The upper suspense boundary would never fire and be unexpectedly claimed by the buried suspense boundary in . That could lead to pretty serious confusion.

I guess you are right. When a component can be suspense, the components need an explicit suspense boundary.

On the other hand, an outer suspense boundary renders first. If the component does not match the current breakpoint, the suspender blocks the inner component from rendering. When a component matches, the suspender does nothing, but one level higher is a suspense boundary declared. If you do not fire throw a promise, the suspense boundary will not block.

To get the correct behaviour including fallbacks, docs need to describe those situations carefully.

I tried a lot of things involving suspense boundaries every time. IΒ΄m pretty convinced there is no other way around it.

Thx!

damassi commented 2 years ago

Thanks for the overview @gurkerl83 πŸ‘

A couple things:

1) One issue is the primary web-app using Fresnel at Artsy will need some fairly major refactoring before we can upgrade to React 18 and until that happens we're effectively blocked. Meaning, I'm hesitant to give the go ahead on this without the ability to test it properly in our codebase, even though it'd be a major version bump.

2) I'll be heading out for some long-ish summer OOO in the coming week and won't be able to properly review a PR.

With these two things, the best I can suggest here is that we fork Fresnel to implement your solution, and that we return to this once I get back. Once we're ready to do a proper integration we can merge the fork back into main.

Again, really appreciate all of your investigation here πŸ™

ayepRahman commented 2 years ago

Hi there, i've recently used the latest next.js which uses react v18. would love to contribute on this

murrowblue22 commented 2 years ago

hey guys any updates? would greatly appreciate any help as I too am impacted by this issue

EmilioAiolfi commented 2 years ago

@damassi or someone have a some news about this problem?

damassi commented 2 years ago

No news unfortunately. Will gratefully accept PRs to fix the problem!

gurkerl83 commented 2 years ago

Have figured it out, I will share something next week!

damassi commented 2 years ago

Amazing πŸ™

gurkerl83 commented 2 years ago

I post the required strategy first. The solution is all about suspense, in combination with a few instruments to suspend unmatched components in the proper order. Please read the comments made in the components carefully and provide feedback.

The components I describe here is based on a custom implementation without using fresnel. I wanted to ensure that fresnel avoids any magic. Fresnel's implementation detail is confusing, especially when stacking several stacked providers.

I made changes to fresnel locally to reflect those changes; the props are slightly different.

All hydration errors are gone! Although all components get pre-rendered on the server, only the relevant component, which matches the current breakpoint, gets mounted on the client, exactly what we wanted.

Some prove this is working; look at the deployed version of our web page, with responsive drawers, titles, and table of contents in place. Look at the console, and you should not see any error.

https://millipede-docs-git-refactor-sitemap-gurkerl83.vercel.app/de/docs/perspective/strategy

You can also verify that the suspense boundaries un- and re-suspend on resize in the components tab of react dev tools. The node names are a bit cryptic because it is an optimised, almost production version.

SuspenseWrapper

import { Media, RenderUtils } from '@app/render-utils';
import { useRouter } from 'next/router';
import React, { FC, Suspense, useEffect, useId, useState, useTransition } from 'react';

import { Suspender } from './Suspender';

export interface SuspenseWrapperProps {
  media: Media;
}

/**
 * Background:
 * The feature available in React 17 for canceling out unused DOM elements got eliminated in React 18.
 * The only option for replicating the behavior is to use Suspense and selectively suspend currently
 * unused components through Suspense wrappers / Suspenders.
 *
 * In the context of the approach pursued by fresnel, a variant of a component set that matches
 * a current breakpoint renders; any unmatched component gets suspended.
 *
 * The task of the suspense wrapper component is to interrupt the suspense component
 * by necessary transition pauses.
 *
 * Important:
 * React 18 requires the use of a transition when a value enters a suspense boundary.
 * A Suspense boundary does not serve an API to suspend embedded components.
 *
 * The only way is to make a commitment to the Suspense boundary, this must be done within
 * a Suspense boundary, see component Suspender and its "active" property.
 *
 * The only way is to trigger a commitment to the suspense boundary within a suspense boundary,
 * reference component Suspender, and its "active" property.
 *
 * Note: The suspense wrapper component should be in user-land and not be part of a library
 * because Next specific logic is involved.
 */
export const SuspenseWrapper: FC<SuspenseWrapperProps> = ({
  media: { active, isPending: activeIsPending },
  children
}) => {
  const id = useId();

  const [, setRerenderSuspended] = useState(false);
  const [rerenderSuspendedIsPending, startRerenderSuspendedTransition] =
    useTransition();

  /**
   * Next specific state
   * Either required because there is a bug within the Nextjs router component or
   * a general requirement by the approach; see the suspender component for more information.
   */
  const { isReady } = useRouter();

  useEffect(() => {
    if (!active && isReady) {
      startRerenderSuspendedTransition(() => {
        setRerenderSuspended(value => !value);
      });
    }
  }, [active, isReady]);

  return (
    !activeIsPending &&
    !rerenderSuspendedIsPending && (
      <Suspense fallback={null}>
        <Suspender id={id} freeze={!RenderUtils.isBrowser() ? false : !active}>
          {children}
        </Suspender>
      </Suspense>
    )
  );
};

Suspender

import { FC, ReactNode, useEffect } from 'react';

import { createWakeable, Wakeable } from './suspense';

export const cache = new Map<string, Wakeable<unknown>>();

export interface SuspenderProps {
  id: string;
  freeze: boolean;
  children: ReactNode;
}

/**
 * The task of the suspender component is to instruct the suspense boundary,
 * depending on the value of the "active" property.
 *
 * An important aspect is that the instruction is not to be executed
 * as a side effect but in the render step.
 *
 * 1. Active - true - return the child elements
 * 2. Active - false - creation and submission of a promise (Promise / Thenable)
 *
 * After the first render
 * 3. Active - false -> true - Promise fulfilment + return of the child elements
 * 4. Active - true -> false - Creation and submission of a promise
 *
 * 5. Active - false -> true -> false - Reuse and discard a promise
 *
 * Stable active
 * 6. Active - true -> true - Return of the child elements
 * 7. Active - false -> false - Reuse and submission of a promise (*1)
 *
 * Important: If a suspense boundary is suspended in the current
 * render step and this should also apply to subsequent render steps,
 * a commitment must be made repeatedly.
 */

export const Suspender: FC<SuspenderProps & { children: any }> = ({
  id,
  freeze,
  children
}) => {
  useEffect(() => {
    return () => {
      cache.delete(id);
    };
  }, []);

  /**
   * If !freeze (the breakpoint matches) return children
   */

  if (!freeze) {
    if (!cache.has(id)) {
      // render children; skip the promise phase (case 1 & 6)
      return children;
    } else if (cache.has(id)) {
      const weakable = cache.get(id);
      // resolve promise previously thrown, render children (case 3)
      weakable.resolve();
      cache.delete(id);
      return children;
    }
  }

  /**
   * If freeze (the breakpoint does not match) throw promise to suspend children / return nothing not even null
   */
  if (freeze) {
    if (!cache.has(id)) {
      // (case 2 & 4)
      const weakable = createWakeable();
      cache.set(id, weakable);
      throw weakable;
    } else if (cache.has(id)) {
      // (case 5 & 7)

      /**
       * Note:
       * In Next, the routing gets blocked here, throwing a promise during a navigation attempt.
       *
       * It is required to fulfill the suspense contract by throwing a promise and making
       * the component suspend even when it was suspended before.
       *
       * The relevant fix is to look when the Next router is ready and see the suspense wrapper
       * component, including the transition rerenderSuspendedIsPending, which blocks the
       * suspense boundary from rendering immediately.
       */

      const weakable = cache.get(id);
      throw weakable;
    }
  }

  // do not return children
};

suspense - not related to reacts suspense component

export const STATUS_PENDING = 0;
export const STATUS_RESOLVED = 1;
export const STATUS_REJECTED = 2;

export type PendingRecord<T> = {
  status: 0;
  value: Wakeable<T>;
};

export type ResolvedRecord<T> = {
  status: 1;
  value: T;
};

export type RejectedRecord = {
  status: 2;
  value: any;
};

export type Record<T> = PendingRecord<T> | ResolvedRecord<T> | RejectedRecord;

// This type defines the subset of the Promise API that React uses (the .then method to add success/error callbacks).
// You can use a Promise for this, but Promises have a downside (the microtask queue).
// You can also create your own "thennable" if you want to support synchronous resolution/rejection.
export interface Thennable<T> {
  then(onFulfill: (value: T) => any, onReject: () => any): void | Thennable<T>;
}

// Convenience type used by Suspense caches.
// Adds the ability to resolve or reject a pending Thennable.
export interface Wakeable<T> extends Thennable<T> {
  reject(error: any): void;
  resolve(value?: T): void;
}
// A "thennable" is a subset of the Promise API.
// We could use a Promise as thennable, but Promises have a downside: they use the microtask queue.
// An advantage to creating a custom thennable is synchronous resolution (or rejection).
//
// A "wakeable" is a "thennable" that has convenience resolve/reject methods.
export function createWakeable<T>(): Wakeable<T> {
  const resolveCallbacks: Set<(value: T) => void> = new Set();
  const rejectCallbacks: Set<(error: Error) => void> = new Set();

  const wakeable: Wakeable<T> = {
    then(
      resolveCallback: (value: T) => void,
      rejectCallback: (error: Error) => void
    ) {
      resolveCallbacks.add(resolveCallback);
      rejectCallbacks.add(rejectCallback);
    },
    reject(error: Error) {
      let thrown = false;
      let thrownValue;
      rejectCallbacks.forEach(rejectCallback => {
        try {
          rejectCallback(error);
        } catch (error) {
          thrown = true;
          thrownValue = error;
        }
      });
      if (thrown) {
        throw thrownValue;
      }
    },
    resolve(value: T) {
      let thrown = false;
      let thrownValue;
      resolveCallbacks.forEach(resolveCallback => {
        try {
          resolveCallback(value);
        } catch (error) {
          thrown = true;
          thrownValue = error;
        }
      });
      if (thrown) {
        throw thrownValue;
      }
    }
  };

  return wakeable;
}

To establish the context, here is the provider and how it is consumed.

Media hook - the utilisation of useTransition is essential, the result includes isPending, which is served through context to the relevant suspense wrappers

import { useMemo, useState, useTransition } from 'react';

import { isBrowser } from '../utils/isBrowser';
import { useIsomorphicLayoutEffect } from './useIsomorphicLayoutEffect';

export const useMedia = (
  query: string,
  onChange?: (matched: boolean) => void,
  initialState: boolean | (() => boolean) = false
): [boolean, boolean] => {
  const [isPending, startTransition] = useTransition();

  const mql = useMemo(() => {
    return isBrowser() ? window.matchMedia(query) : null;
  }, [query]);

  const [matched, setState] = useState(mql ? mql.matches : initialState);

  useIsomorphicLayoutEffect(() => {
    if (mql) {
      const onMediaChange = () => {
        const matched = mql.matches;

        startTransition(() => {
          setState(matched);
        });

        onChange && onChange(matched);
      };

      mql.addEventListener('change', onMediaChange);

      return () => {
        mql.removeEventListener('change', onMediaChange);
      };
    }
  }, [mql, onChange]);

  return [matched, isPending];
};

MediaProvider

import { HooksUtils } from '@app/render-utils';
import { createContext, FC, ReactNode } from 'react';

import { Media } from '../../types';
import { Boundary, useTheme } from './use-theme';

const defaultValue: Media = {
  active: true,
  isPending: false,
  className: ''
};

interface MediaMap {
  [key: string]: Media;
}

const MediaContext = createContext<{
  media: MediaMap;
}>({
  media: {
    mobile: defaultValue,
    desktop: defaultValue
  }
});

export const MediaProvider: FC<{ children: ReactNode }> = ({ children }) => {
  const [activeMobile, isPendingMobile] = HooksUtils.useMedia(
    'only screen and (max-width : 900px)'
  );
  const classNameMobile = useTheme(Boundary.upper);

  const [activeDesktop, isPendingDesktop] = HooksUtils.useMedia(
    'only screen and (min-width : 900px)'
  );
  const classNameDesktop = useTheme(Boundary.lower);

  return (
    <MediaContext.Provider
      value={{
        media: {
          mobile: {
            active: activeMobile,
            isPending: isPendingMobile,
            className: classNameMobile
          },
          desktop: {
            active: activeDesktop,
            isPending: isPendingDesktop,
            className: classNameDesktop
          }
        }
      }}
    >
      {children}
    </MediaContext.Provider>
  );
};

export const MediaConsumer = MediaContext.Consumer;

Consumer

return (
    <>
      <MediaConsumer>
        {({ media: { mobile, desktop } }) => {
          return (
            <>
              <SuspenseWrapper media={mobile}>
                <MobileDrawer
                  sx={{
                    gridArea: 'app-left'
                  }}
                  className={mobile.className}
                >
                  <DrawerHeader />
                  <Divider variant='middle' />
                  {tree}
                </MobileDrawer>
              </SuspenseWrapper>
              <SuspenseWrapper media={desktop}>
                <DesktopDrawer
                  sx={{
                    gridArea: 'app-left'
                  }}
                  className={desktop.className}
                >
                  <DrawerHeader />
                  <Divider variant='middle' />
                  {tree}
                </DesktopDrawer>
              </SuspenseWrapper>
            </>
          );
        }}
      </MediaConsumer>
    </>
  );

Note: The suspender component can be simplified. Also, you can use a library called resuspend, but its internals is more complex. It will work the same as the implementation of the suspender component provided, but the suspense wrapper and its signals of pending transitions are also essential.

Thx!

damassi commented 2 years ago

Hi @gurkerl83 - I will take a look at this today!

gurkerl83 commented 2 years ago

Hi @gurkerl83 - I will take a look at this today!

Hi @damassi - A few things I have not posted is the className generator respectively, the use-theme hook, but this is not required to understand the overall strategy.

damassi commented 2 years ago

@gurkerl83 - would you mind posting a Gist with all of the code? I'll put it together over here and see how things work in our app. Without having this run locally it will be hard to say.

gurkerl83 commented 2 years ago

@damassi I will create a git repo but no library; for reasons of time it has to wait until tomorrow. Have you looked up the link to an almost production version of the concept? Did you experience an error in the console? I hope not!

https://millipede-docs-git-refactor-sitemap-gurkerl83.vercel.app/de/docs/perspective/strategy

damassi commented 2 years ago

No need to create a whole git repo! Can even update the comment above with the full code. Just something with all of the moving parts and i'll take it from there.

gurkerl83 commented 2 years ago

@damassi I have created the repo; hopefully, I have not overlooked anything. The sources are in the lib folder.

Important: The styles rely on sass (dev dependency). When you test the approach in your app, you must also install sass. Tomorrow I will add a small Next demo.

 "dependencies": {
    "next": "12.2.4", => latest Next
    "react": "0.0.0-experimental-ca990e9a7-20220804", => latest experimental but 18.2.0 should also work, not tested with 18.1.0 and older
    "react-dom": "0.0.0-experimental-ca990e9a7-20220804", => latest experimental but 18.2.0 should also work, not tested with 18.1.0 and older
  }

https://github.com/gurkerl83/fresnel-alternative

Thx!

gurkerl83 commented 2 years ago

Further testing does not show the reported NextJs anymore. Not sure what happened. I really can not reconstruct it anymore. The only thing I did was update my MacOS version to 12.5, but this is super wired!

damassi commented 2 years ago

@gurkerl83 - I spent a day trying to work in your suspender and suspense components and was unfortunately unable to get it working with our current API. It's honestly a bit over my head! If you have some time to open a PR to adapt your solution into our lib that would be helpful.

In the meantime, for those reading, the best we can do to for React 18 SSR support is to use the disableDynamicMediaQueries prop as applied to the context provider:

<MediaContextProvider disableDynamicMediaQueries>
  ...
</MediaContextProvider>

It will work but note that it will fire all effects for all child components within individual <Media> wrappers (all breakpoints). It's not ideal, but then again it might work just fine for most use-cases.

I will push up a PR updating the lib / types to React 18 now (done).

gurkerl83 commented 2 years ago

@damassi Solved the integration problem; I will provide a PR later today. Please do not merge it immediately because we have to discuss the Media component child options. Right now, only the render prop variant is possible.

gurkerl83 commented 2 years ago

@damassi The PR is here, when you have time please have a look, Thx!

https://github.com/artsy/fresnel/pull/289

damassi commented 2 years ago

Will move the convo over there πŸ‘€

damassi commented 2 years ago

See this PR: https://github.com/artsy/fresnel/pull/290

damassi commented 2 years ago

Update: After some further testing we noticed some issues with Next, specifically the error

This Suspense boundary received an update before it finished hydrating

I'm not sure why this was not appearing during testing of #290, but out of caution we backed out of the suspense integration, deprecated v5, published a v6 (πŸ˜“) and are officially suggesting that folks use the approach as outlined in the README:

Due to React 18 more strictly handling server-side rendering rehydration full SSR support is incompatable with any version greater than 17. See https://github.com/artsy/fresnel/issues/260 to track our progress in fixing this issue, and Dan Abromov's https://github.com/facebook/react/issues/23381#issuecomment-1096899474.

In the meantime -- for users of server-side rendering features -- you can disable DOM cleaning optimizations by setting disableDynamicMediaQueries on the context like so and things should work:

<MediaContextProvider disableDynamicMediaQueries>
  <Media at='xs'>
    <MobileApp />
  </Media>
  <Media greaterThan='xs'>
    <DesktopApp />
  </Media>
<MediaContextProvider>

Note that this will fire all effects within sub components for each breakpoint on re-hydration. For some users this could be an issue; for many others, no problem at all.

Many apologies for any confusion this may have caused. We should have tested this more thoroughly on our end before publishing a major version update.

gurkerl83 commented 2 years ago

@damassi I do not see such an error in my project. I use the latest Next version.

The project’s sources are here. https://github.com/project-millipede/millipede-docs

The project is currently not using fresnel, although it is using the same things integrated in v5.

Due to constant improvements on Suspense the React team is doing we use a nightly build of react see the package.json file in the project, maybe it helps.

Can you explain why v5 is deprecated, because the commits from v5 forward are empty, only the major version number was increased?

damassi commented 2 years ago

Can you explain why v5 is deprecated, because the commits from v5 forward are empty, only the major version number was increased?

I reset the codebase back to the state it was in here https://github.com/artsy/fresnel/pull/287, just before the suspense work was integrated. While the above doesn't solve the issue of the multiple effects firing for different breakpoints, it's "good enough" for many and perhaps most use-cases until a formal fix is in.

I'll continue to poke at your solution above, and I'm still (very) uncertain about what I was looking at when testing v5. I had yarn linked it to our next codebase during integration and didn't see any issues until publishing, at which point the suspense boundary hydration appeared appeared, both from the npm package itself and linking internally to the lib. I also tested it thoroughly in the SSR example app.

Maybe I landed in some kind of Philip K Dick alternative timeline or something, because it's certainly weird.

Sagerahl commented 2 years ago

Hi, jumping into the discussion to say that we also have this error with version 5.0.0 and 5.0.1. This suspense Boundary Received an Update before it finished Hydrating Our project use Next.js v12.2.5 and React v18.2.0. Tell me if you want more information or if I can help carry out tests.

gurkerl83 commented 2 years ago

@Hadrien-Segeral Can you please share some code. Are you passing props in the component you use within the media component?

Sagerahl commented 2 years ago

I was in the process of producing a minimal reproducible example and found that the error is due to the use of: useMediaQuery from @mui/material (v5.10.0) which I assume conflicts with Fresnel version 5.0.x.

What's surprising is that using useMediaQuery anywhere in the code, whether in the root of the pages in _app.tsx or in a child component unrelated to Fresnel, causes the error to pop up. Here is a code sample:

// _app.tsx
import type { AppProps } from 'next/app'
import { useMediaQuery } from '@mui/material'
import Footer from '../components/footer/footer'

export default function MyApp({ Component, pageProps }: AppProps) {

  // Retrieves the user's browser color scheme preference
  const prefersDarkMode = useMediaQuery('(prefers-color-scheme: dark)');

  return (
    <>
        <Component {...pageProps} />
        <Footer />
    </>
  )

}
// footer.tsx
import { useMediaQuery } from '@mui/material'
import { Media, MediaContextProvider } from '../../media'

export default function Footer() {
    // const anotherUseOfMediaQuery = useMediaQuery('');
    return (
        <footer>
            <MediaContextProvider>
                <Media at="sm">
                    <div>Hello Mobile</div>
                </Media>
                <Media greaterThan="sm">
                    <div>Hello Desktop</div>
                </Media>
            </MediaContextProvider>
        </footer>
    )
}
gurkerl83 commented 2 years ago

I remember using the useMediaQuery hook from MUI related to the problem mentioned. Not sure why; maybe it is a bug, but the hook implementation looks rather complex. I have tested several alternatives, and most of them seem to work.

gurkerl83 commented 2 years ago

I have no cycles or energy left for this topic. I have provided a working solution, and that's it. Maybe the React team will decide in the future to make Suspense for this use case simpler, but in the meantime, I will ignore any activity here.

Sagerahl commented 2 years ago

I'll investigate the hook code to see what could be causing this. Otherwise I will choose an alternative. Thank you for all the amazing work provided @gurkerl83 and @damassi πŸ™

damassi commented 2 years ago

As a side-note, that Next.js hydration error can happen in a few different ways. We just ran into it in an unrelated area of the codebase around navigation transitions between pages. IMHO, React has created a complexity monster with Suspense, and they should have broken concurrency features out into a separate library. Maybe the DX will improve in a future version.

Geoff-Ford commented 2 years ago

Was @gurkerl83's solution the only known potential fix? And do I understand correctly that this fix does work, just not when combined with MUI's useMediaQuery? (I'm assuming @damassi's comment above regarding other issues is referring to general issues with Suspense and not unique to using Fresnel with React v18)

Just curious if @gurkerl83's fix would help more Fresnel users than it would hinder? Is it better for all Next.js/Fresnel users to be stuck at React v17 vs MUI useMediaQuery users being stuck at Fresnel v4?

gurkerl83 commented 2 years ago

We also use MUIs useMediaQuery combined with the solution provided and released in v5. I think enabling the noSsr option is essential; we do not experience problems with this setup.

const isMobile = () => {
  const theme = useTheme();
  return useMediaQuery(theme.breakpoints.down('md'), {
    noSsr: true
  });
};

https://github.com/project-millipede/millipede-docs/blob/9337bfce5ad32024e9643a794c012d57a357e27a/packages/app/layout/src/components/toolbar/HideOnScroll.tsx#L20

@Geoff-Ford I still believe the solution I have provided is the only one currently working. From a theoretical perspective and learned from discussions, I have not seen any other approach forward.

Maybe this helps. Thx!

Geoff-Ford commented 2 years ago

Thanks for confirming @gurkerl83. So I can lock Fresnel at v5 (not ideal for any future changes) and move forward with React v18+. Let's hope this use case gets more consideration from the React team in the future.