How does this work with the new Remix layout export in root.jsx?

684efs3 commented 4 months ago

Describe the bug

I am unable to create a Layout in root.jsx, as per here and make Remix-Themes work.

Your Example Website or App

n/a

Steps to Reproduce the Bug or Issue

n/a

Expected behavior

I used useRouteLoaderData() instead of useLoaderData for const data = useRouteLoaderData(), but this still results in the error:

Error: useTheme must be used within a ThemeProvider

Screenshots or Videos

No response

Platform

Mac on Chrome

Additional context

No response

abereghici commented 4 months ago

Hi @684efs3! I'm currently working on a new version to support new remix layout.

abereghici commented 4 months ago

@684efs3 Meanwhile, we could not export the Layout, but wrap the App, ErrorBoundary and HydrateFallback with it.

I created an example here: https://github.com/abereghici/vite-remix-themes/blob/main/app/root.tsx https://github.com/abereghici/vite-remix-themes/blob/main/app/routes/counter.tsx

TheAifam5 commented 4 months ago

Has something changed? Even with using your approach, I still get :

Error: useTheme must be used within a ThemeProvider
Error: useTheme must be used within a ThemeProvider
You cannot `useLoaderData` in an errorElement (routeId: root)
Error: useTheme must be used within a ThemeProvider

abereghici commented 4 months ago

@TheAifam5 I updated the vite-remix-themesrepo and now I'm using useRouteLoaderData("root") instead of useLoaderData so we can use the theme in errorElements too. I don't have the provider error in my example. Could you please share your setup?

TheAifam5 commented 4 months ago

@abereghici I have used everything what you've had in this repository, but I see a mistake. Based on a Remix Documentation, the children in Layout will be the root Component (like App), ErrorBoundary, or HydrateFallback. Each rendered page will be like this:

Layout -> App / ErrorBoundary / HydrateFallback -> Providers -> ThemeProvider -> Layout -> Outlet / DefaultErrorBoundary / H1

The problem is with useTheme being in Layout, so I have modified it a little:

Layout -> ThemeProvider -> InnerLayout -> App / ErrorBoundary / HydrateFallback -> Outlet / DefaultErrorBoundary / H1

Here, I moved useTheme to InnerLayout because it is required to be within a ThemeProvider context.

Before:

export function Layout({ children }: { children: React.ReactNode }) {
  const data = useRouteLoaderData<typeof loader>('root');
  const [theme] = useTheme();
  const nonce = useNonce();

  return (
    <html lang="en" data-theme={theme} className={theme ?? ''}>
      <head>
        <meta charSet="utf-8" />
        <meta name="viewport" content="width=device-width, initial-scale=1" />
        <Meta />
        <Links />
      </head>
      <body
        className="bg-white text-black dark:bg-black dark:text-white"
        suppressHydrationWarning
      >
        {children}
        <ScrollRestoration nonce={nonce} />
        <PreventFlashOnWrongTheme ssrTheme={Boolean(data?.theme)} />
        <Scripts nonce={nonce} />
      </body>
    </html>
  );
}

export default function App() {
  return (
    <Providers>
      <Outlet />
    </Providers>
  );
}

export function ErrorBoundary() {
  return (
    <Providers>
      <DefaultErrorBoundary />
    </Providers>
  );
}

export function HydrateFallback() {
  return (
    <Providers>
      <h1>Loading...</h1>
    </Providers>
  );
}

function Providers({ children }: { children: React.ReactNode }) {
  const data = useLoaderData<typeof loader>();
  return (
    <ThemeProvider
      specifiedTheme={data?.theme as Theme}
      themeAction="/resources/set-theme"
    >
      <Layout>{children}</Layout>
    </ThemeProvider>
  );
}

After:

export function Layout({ children }: { children: React.ReactNode }) {
  const data = useRouteLoaderData<typeof loader>('root');

  return (
    <ThemeProvider
      specifiedTheme={data?.theme as Theme}
      themeAction="/resources/set-theme"
    >
      <InnerLayout ssrTheme={Boolean(data?.theme)}>{children}</InnerLayout>
    </ThemeProvider>
  );
}

export default function App() {
  return <Outlet />;
}

export function ErrorBoundary() {
  return <DefaultErrorBoundary />;
}

export function HydrateFallback() {
  return <h1>Loading...</h1>;
}

function InnerLayout({
  ssrTheme,
  children,
}: {
  ssrTheme: boolean;
  children: React.ReactNode;
}) {
  const [theme] = useTheme();
  const nonce = useNonce();

  return (
    <html lang="en" data-theme={theme} className={theme ?? ''}>
      <head>
        <meta charSet="utf-8" />
        <meta name="viewport" content="width=device-width, initial-scale=1" />
        <Meta />
        <Links />
      </head>
      <body
        className="bg-white text-black dark:bg-black dark:text-white"
        suppressHydrationWarning
      >
        {children}
        <ScrollRestoration nonce={nonce} />
        <PreventFlashOnWrongTheme ssrTheme={ssrTheme} nonce={nonce} />
        <Scripts nonce={nonce} />
      </body>
    </html>
  );
}

abereghici commented 4 months ago

@TheAifam5 Nice! I'm glad it worked! I'll update my repository with your changes, it makes more sense.

OnlyLogiicC commented 4 months ago

Hi ! You will be glad to hear that with those changes, remix-themes works with the new SingleFetch API ! I was getting an error with the previous placement of the ThemeProvider.

My resources.set-theme.tsx is as follow :

import { unstable_defineAction as defineAction } from "@vercel/remix";
import { createThemeAction } from "remix-themes";
import { themeSessionResolver } from "~/.server/sessions";

export const action = defineAction(createThemeAction(themeSessionResolver))

bravo-kernel commented 3 months ago

Following the code provided by @TheAifam5 everything works as expected with one exception: "real" 404 errors do not pick up on darkmode (with cookie present and working for all other pages including thrown 404s). Any pointers or is this expected behavior?

TheAifam5 commented 3 months ago

@bravo-kernel can you make a sample? What you mean by "real" 404?

bravo-kernel commented 3 months ago

Apologies if I am using the wrong terminology but by "real 404" I mean any non-existing route.

An example of such a non-existing route: currently looking at http://localhost:3000/adm/sd/sd/sd in light-mode (where the rest of my app is in dark-mode)
The ones that do pick up on the theme are e.g. dynamic routes with an invalid id where I just return a 404 using json response, those show up in dark-mode as expected. E.g. http://localhost:3000/bicycles/some-dummy-id

bravo-kernel commented 3 months ago

@TheAifam5 I have created https://stackblitz.com/edit/remix-run-remix-dtxaef that hopefully shows what I mean. All pointers welcomed.

TheAifam5 commented 3 months ago

@bravo-kernel your example was wrong.

https://remix.run/docs/en/main/file-conventions/routes#root-route:

The file in app/root.tsx is your root layout, or "root route" (very sorry for those of you who pronounce those words the same way!). It works just like all other routes, so you can export a [loader](https://remix.run/docs/en/main/route/loader), [action](https://remix.run/docs/en/main/route/action), etc.

The root route typically looks something like this. It serves as the root layout of the entire app, all other routes will render inside the [<Outlet />](https://remix.run/docs/en/main/components/outlet).

If you really want to have a Layout as a component, then you need to re-export it in the root.tsx. You have used Layout in App, ErrorBoundary and HydrateFallback which is wrong - those are children of Layout, take a look what I did in my example above.

Now, here is corrected example that works: https://stackblitz.com/edit/remix-run-remix-nrmf21?file=app%2Froot.tsx

What I have done:

removed <Layout>...</Layout> usage.
re-exported Layout so that is a root layout for all pages (you should be able to define nested layouts, but those still will be inside a root layout)
route for setting theme wasnt triggering the change, fixed that too.

It looks like that the ErrorBoundary cannot access root route loader data. The useRouteLoaderData always returns undefined.

What you can do, you could use local-storage to backup last theme, but it will flash from dark to light and otherwise.

bravo-kernel commented 3 months ago

@TheAifam5 This is amazing. It all makes sense now with the added explanation about root and layout.

Thanks a million for taking the time to fix the example. I am sure other end users (like myself) will benefit from it in the future. ♥

bravo-kernel commented 3 months ago

There is one issue left with the sandbox. If manually triggering a 404, it shows FOUC due to a hydration error.

To reproduce:

Set theme to dark using the top-most link
Then manually update the URL of the container to e.g. container.url/trigger/fouc and enter
See light theme flicker before switching to dark theme
Check console to see the hydration error message

bravo-kernel commented 3 months ago

Ah, missed your note. Seems this is to be expected 😢

It looks like that the ErrorBoundary cannot access root route loader data. The useRouteLoaderData always returns undefined.

What you can do, you could use local-storage to backup last theme, but it will flash from dark to light and otherwise.

bravo-kernel commented 3 months ago

There is a solution.

Adding below splat route routes/$.tsx solves the FOUC/hydration errors.

https://stackblitz.com/edit/remix-run-remix-jnyzn7

I think this ticket can be closed as completed.

/**
 * Splat route to fix 404 FOUC/hydration error
 * 
 * @link {@see https://github.com/remix-run/remix/discussions/5186#discussioncomment-4748778}
 */ 

import { json } from "@remix-run/node";

/**
 * Create a response receiving a JSON object with the status code 404.
 * @example
 * export async function loader({ request, params }: LoaderArgs) {
 *   let user = await getUser(request);
 *   if (!db.exists(params.id)) throw notFound<BoundaryData>({ user });
 * }
 */
function notFound<Data = unknown>(
  data: Data,
  init?: Omit<ResponseInit, "status">,
) {
  return json<Data>(data, { ...init, status: 404 });
}

export function loader() {
  throw notFound(null);
}

export default function NotFound() {
  return null;
}

newdisease commented 2 months ago

Are there any updates on this problem? I understand that I can use temporary solutions, but the library description doesn't mention this problem, and the new Remix has a different structure.

jikyu commented 3 weeks ago

Anyone getting Prop "data-theme" did not match warnings in console with the work-around from @TheAifam5 ?

hook.js:608 Warning: Prop `data-theme` did not match. Server: "null" Client: "dark" Error Component Stack
    at html (<anonymous>)
    at InnerLayout (root.tsx:29:3)
    at ThemeProvider (remix-themes.js?v=7f6bf165:105:30)
    at Layout (root.tsx:59:26)
    at RenderedRoute (@remix-run_react.js?v=7f6bf165:407:5)
    at RenderErrorBoundary (@remix-run_react.js?v=7f6bf165:367:5)
    at DataRoutes (@remix-run_react.js?v=7f6bf165:1400:5)
    at Router (@remix-run_react.js?v=7f6bf165:752:15)
    at RouterProvider (@remix-run_react.js?v=7f6bf165:1215:5)
    at RemixErrorBoundary (@remix-run_react.js?v=7f6bf165:2859:5)
    at RemixBrowser (@remix-run_react.js?v=7f6bf165:4409:46)

edgars-sirokovs commented 3 weeks ago

Hi @684efs3! I'm currently working on a new version to support new remix layout.

Hi, @abereghici! Thanks for the work you've done! This package has been an amazing time saver for me.

Are there any updates regarding the Layout export? I see some workarounds/solutions above but I'd like to see the "official" approach documented in the readme.

abereghici / remix-themes