Catch and Error Boundaries are rendered twice

designbyadrian commented 1 year ago

I've copied the Remix.run basic example to reproduce the issue:

https://github.com/designbyadrian/remix-island-test

In short: CatchBoundary and ErrorBoundary, at least on root level, will render twice, with the contents of head in two places in the DOM.

kiliman commented 1 year ago

Interesting. Yeah, I didn't test all the possibilities when I created the POC.

Does this only happen when the Catch/Error boundaries are triggered on the server?

designbyadrian commented 1 year ago

@kiliman Yes, it seems so. In my linked example, navigating with the element (to a page that doesn't exist) works as expected.

kiliman commented 1 year ago

Ok, I figured out the issue. The error or thrown Response is stored in remixContext.staticHandlerContext (as well as serverHandoffString).

I updated the context switch to strip the errors out when rendering <Head>. The second render will properly render errors.

I fixed it in my POC. https://github.com/kiliman/remix-hydration-fix/commit/5e71c482d5041ec0826737b863bd1f3774e9bcaf

Here's a sample https://remix-hydration-fix-production.up.railway.app

export function switchRootComponent(
  remixContext: EntryContext,
  Head: RouteComponent
): EntryContext {
  let serverHandoffString = remixContext.serverHandoffString;
  if (serverHandoffString) {
    let serverHandoff = JSON.parse(serverHandoffString);
    // remove errors from JSON string
    delete serverHandoff.state.errors;
    serverHandoffString = JSON.stringify(serverHandoff);
  }

  return {
    ...remixContext,
    serverHandoffString,
    staticHandlerContext: {
      ...remixContext.staticHandlerContext,
      errors: null, // remove errors from context
    },
    routeModules: {
      ...remixContext.routeModules,
      root: {
        ...remixContext.routeModules.root,
        default: Head,
      },
    },
  };
}

When I tried in remix-island, I'm getting an odd error that I don't have time to track down.

Cannot read properties of null (reading 'useState')
TypeError: Cannot read properties of null (reading 'useState')
    at useState (/Users/michael/Projects/oss/remix-island/node_modules/react/cjs/react.development.js:1622:21)
    at Head (/Users/michael/Projects/oss/remix-island/src/index.ts:24:35)
...

designbyadrian commented 1 year ago

I appreciate your enthusiasm to find a solution! I'll implement yours in a day or so and try it out.

designbyadrian commented 1 year ago

@kiliman I can confirm that your updates work as intended! You're a star! ⭐

Xiphe commented 1 year ago

Added the fix from @kiliman to the lib and released as v0.1.1

Feels a little much like piggybacking on your solutions Michael... let me know if you'd like to move the project into your namespace

kiliman commented 1 year ago

@Xiphe no problem. Thanks for making an effort to package it up. I've already got enough packages to support 😅

@designbyadrian glad it's working! 🥳

Xiphe / remix-island

Catch and Error Boundaries are rendered twice #1