fix(remix-react): An error in MetaFunction or LinkFunction should render ErrorBoundary

[mcansh]

Signed-off-by: Logan McAnsh logan@mcan.sh

Closes #3140

• Docs
• Tests

Testing Strategy:

• these tests cover the changed code: error-boundary-test.ts
• I also opened up a new playground to make sure it all worked there as well.

[kentcdodds]

I think I must be missing something here, but I don't see any behavioral changes here. I'm pretty sure the code does exactly the same thing before and after.

[mcansh]

little more digging, looks like if you have a custom ErrorBoundary we render the generic Unexpected Server Error

but if you don't have one, it renders the default RemixErrorBoundary

[changeset-bot]

⚠️ No Changeset found

Latest commit: 6e57656

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[brophdawg11]

I wonder if there's any use in distinguishing errors thrown when calling a route meta() function here from errors thrown when rendering the <meta> tags. The rendering is far more in our control and we should be able to be pretty confident it won't throw render errors. The exported meta function contents are out of our control - but we do control the execution of it in our own <Meta> component.

Spitballing here but what if we did something like this in <Meta>

for (let match of matches) {
  let routeId = match.route.id;
  try {
    let routeMeta = routeModule.meta({ data, parentsData, params, location })
    ...
  } catch (error) {
    // Re-throw the error and associate it with the routeId that threw it
    throw new MetaError(routeId, error);
  }
}

Then we could identify the error in this handleDocumentRequest catch block as a meta-specific error (via instanceof MetaError) and know the routeId that caused it and look for the nearest ancestor boundary (i.,e., prevent re-calling the problematic meta on the second pass - but attempt to call the successful ones above it).

This is effectively what we're doing with the mutable boundaryIds (identifying how deep we got) - but we have no choice there since it's React calling them, not us. meta isn't subject to that same restriction so we could be a bit more direct in the identification of the problematic routeId.

[brophdawg11]

Actually we'd still need some logic changes in Meta to do that too I think. It doesn't have the concept of renderableMatches that we have for getDocumentHeaders which I think is what it would need to not call child's meta function on the second pass if we had identified parent as the renderable boundary.

[brophdawg11]

Closing for now, can re-open if/when we come back to this.