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

Breaking change with latest React experimental builds #260

Closed
gurkerl83 opened this issue Feb 18, 2022 · 87 comments · Fixed by #290 or #341
Closed

Breaking change with latest React experimental builds #260

gurkerl83 opened this issue Feb 18, 2022 · 87 comments · Fixed by #290 or #341
Labels

Comments

@gurkerl83
Copy link

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
facebook/react@848e802

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

Thx!

@damassi
Copy link
Member

damassi commented Feb 18, 2022

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
Copy link
Author

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.

facebook/react@3f5ff16

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

  • Remove throwing errors altogether; at least make it optional because the third argument in hydrateRoot is not used/implemented by any consumer of this API, such as in NextJS, although they promise you can use the latest experimental React version
  • Disable enableClientRenderFallbackOnHydrationMismatch when suppressHydrationWarning is set.

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

Thx!

@damassi
Copy link
Member

damassi commented Feb 28, 2022

Thank you @gurkerl83 for the digging 🙏

@KrustyC
Copy link

KrustyC commented Apr 1, 2022

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
Copy link
Author

gurkerl83 commented Apr 1, 2022

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.

facebook/react#23381 (comment)

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
Copy link
Member

damassi commented Apr 1, 2022

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

@alloy
Copy link
Collaborator

alloy commented Apr 12, 2022

@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
Copy link
Author

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

  • Media creates a dedicated div
  • Render-props.

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
Copy link
Member

damassi commented Apr 14, 2022

Awesome @gurkerl83! Thank you for digging into this 🙏

@gurkerl83

This comment was marked as outdated.

@KarthikeyanRanasthala
Copy link

Any update on this? :(

@gurkerl83
Copy link
Author

gurkerl83 commented May 6, 2022

Any update on this? :(

Unfortunately not :(

@damassi
Copy link
Member

damassi commented May 6, 2022

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

@JensMou
Copy link

JensMou commented Jun 1, 2022

Any updates on this? Or an alternative, that is not pure css? 🙏

@gurkerl83
Copy link
Author

gurkerl83 commented Jun 6, 2022

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
Copy link
Member

damassi commented Jun 6, 2022

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
Copy link
Author

gurkerl83 commented Jun 6, 2022

@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
Copy link
Member

damassi commented Jun 6, 2022

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
Copy link

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

@murrowblue22
Copy link

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

@EmilioAiolfi
Copy link

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

@damassi
Copy link
Member

damassi commented Jul 26, 2022

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

@gurkerl83
Copy link
Author

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

@damassi
Copy link
Member

damassi commented Aug 7, 2022

Amazing 🙏

@gurkerl83
Copy link
Author

gurkerl83 commented Aug 9, 2022

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!

@alyustik
Copy link
Contributor

I still have hydration errors if i remove disableDynamicMediaQueries @ react 18.2.0, next 13.4.13 within app folder, fresnel 6.2.0

@damassi
Copy link
Member

damassi commented Aug 29, 2023

Ah, we're not using the new app folder over here due to unfortunate publicRuntimeConfig depreciations and docker. @boylec - are you using layouts?

@boylec
Copy link

boylec commented Aug 29, 2023

Ah, we're not using the new app folder over here due to unfortunate publicRuntimeConfig depreciations and docker. @boylec - are you using layouts?

no we are not yet using the new app folder or the layouts functionality therein

@alyustik
Copy link
Contributor

alyustik commented Aug 29, 2023

I've tried to roll back my project to pages directory and still no luck. Then I have cloned this example https://github.com/artsy/fresnel/tree/main/examples/nextjs, have removed disableDynamicMediaQueries and updated next version to 13.4.7 and i still have hydration errors

@damassi
Copy link
Member

damassi commented Aug 29, 2023

Ugh, never ending. I just tried this in the example app, and yup, upgrading everything, and still seeing the hydration error. No clue. Our two fairly vanilla next apps work great, with no issues at all and we're using this feature all over.

As usual, swamped at work but will try to find a moment to look more into this. Going to revert the other PR in the meantime.

@boylec
Copy link

boylec commented Aug 29, 2023

I too was able to reproduce using the example.

We use Media components quite robustly as well across our entire app - which is not a simple app - with no hydration issues.

Not sure what the difference is between the example app and my own app sorry for the false hopes folks.

@damassi
Copy link
Member

damassi commented Aug 29, 2023

Yeah, the one thing i noticed that was different between our apps and the example with the use of _app. I even tried adding that here, but no go.

@Geoff-Ford
Copy link

Does this mean that so long as you are still using pages, we can use latest React and Next without issue?
I have no intention of switching to app as site is predominantly SSG.

@damassi
Copy link
Member

damassi commented Aug 29, 2023

@Geoff-Ford - We have a few examples of this working, as if there was never a problem at all. Artsy has a few next.js apps that have been spun up recently and we use fresnel with no problem. Boylec, too. Unfortunately, trying to update the example app and we still see the hydration error. So there's some mystery here. I would suggest trying it out on your end with latest next and react and react-dom and reporting back here.

@Geoff-Ford
Copy link

I will hopefully have some time this week to try this out and sure, will report back. Thanks.

@damassi damassi reopened this Aug 29, 2023
@gurkerl83
Copy link
Author

In my view, the issue of hydration errors persists despite the advancements in React 18.2, as well as in Canary and Next.js 13. These updates haven't effectively addressed the problem. The root of the issue lies deep within React's architecture, and it appears to be an intentional design choice, leaving us with limited options for a fix. You may not be encountering hydration errors as frequently due to less stringent client fallback mechanisms, but the underlying issue remains unresolved. Thx!

@Geoff-Ford
Copy link

Unfortunately I also still see the hydration errors after updating react to v18.2.0 😞

@mgreenw
Copy link
Contributor

mgreenw commented Nov 2, 2023

Just put up a PR that I hope resolves this issue! Check it out: #341

@artsyit
Copy link
Contributor

artsyit commented Nov 2, 2023

🚀 Issue was released in v7.0.0 🚀

@artsyit
Copy link
Contributor

artsyit commented Nov 2, 2023

🚀 Issue was released in v7.0.0 🚀

@damassi
Copy link
Member

damassi commented Nov 2, 2023

Pretty sure this has been definitely fixed in 7.1.3 (for Next.js users specifically, as tested in the examples folder). Can y'all run:

yarn install @artsy/fresnel@latest

and report back?

@damassi
Copy link
Member

damassi commented Nov 2, 2023

cc @Geoff-Ford, @boylec, @alyustik

@alyustik
Copy link
Contributor

alyustik commented Nov 3, 2023

checked on both pages and app routers - works like charm <3

@damassi
Copy link
Member

damassi commented Nov 3, 2023

Well then: CASE CLOSED! 🍻 Thanks again to @mgreenw for his brilliantly simple solution to the problem.

Folks, keep an eye out as you use it and file any issues that might come up 👍

@Geoff-Ford
Copy link

Happy days!!! Can confirm all good! Such a relief as this was always at the back of my mind every time I read about new react/nextjs features that I couldn't update to.

@pawanrana1992
Copy link

checked on both pages and app routers - works like charm <3

@alyustik @Geoff-Ford thanks for resolutions, And I need a help to integrate this @artsy/fresnel library in nextjs@14, I don't found any example or configuration which can run the library smoothly in nexjs app directory. Could you pls help me get example configuration to configure the @artsy/fresnel inside nextjs@14 with tailwindcss if possible. I am stuck here and I need to move this any how, please help. thanks in advance.

@damassi
Copy link
Member

damassi commented Nov 7, 2023

@pawanrana1992 - if there's an issue with Next 14, please file a new issue and be detailed in what problem you're facing 🙏

@pawanrana1992
Copy link

@damassi sorry if miss guide you here, Actually I need setup/configuration inside app routing approach using nextjs@14. inside @artsy/fresnel I can only see pages routing example. Could you please help get the example project or doc to use @artsy/fresnel inside nextjs@14 or with app routing please.

@alyustik
Copy link
Contributor

alyustik commented Nov 7, 2023

@alyustik @Geoff-Ford thanks for resolutions, And I need a help to integrate this @artsy/fresnel library in nextjs@14, I don't found any example or configuration which can run the library smoothly in nexjs app directory. Could you pls help me get example configuration to configure the @artsy/fresnel inside nextjs@14 with tailwindcss if possible. I am stuck here and I need to move this any how, please help. thanks in advance.

I can share my boilerplate, maybe it will help you (no ts and no tailwind there unfortunately): https://github.com/alyustik/next_boilerplate/

@damassi
Copy link
Member

damassi commented Nov 7, 2023

Thats a good point about setting up an app-directory example. I can do that 👍 (but likely won't be today)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet