search

solidjs / solid-router

A universal router for Solid inspired by Ember and React Router
MIT License
1.15k stars 147 forks source link

Routing breaks if a route component's root is a `<Show>` for a rejected resource #475

Open spiffytech opened 2 months ago

spiffytech commented 2 months ago

Describe the bug

In the Stackblitz, a component uses <Show> to only show content once a resource has loaded. If that resource's Promise rejects, all routes cannot render anymore. Navigation will invoke a component function, but its body is never displayed.

Your Example Website or App

https://stackblitz.com/edit/solidjs-templates-eebkn9?file=src%2Findex.tsx

Steps to Reproduce the Bug or Issue

  1. Create a resource which rejects
  2. Write a component whose top-level element is <Show when={thatResource()}>...

If the <Show> is wrapped in a <div>, this issue does not reproduce.

Expected behavior

I can still navigate to other routes like normal

Screenshots or Videos

No response

Platform

Additional context

No response

ryansolid commented 2 months ago

We throw errors on read with resources. We don't error on fetch. The mechanism similar to Suspense is based around whether or not we are able to render with what we have. So if there isn't an ErrorBoundary that is expected.

spiffytech commented 2 months ago

Ah, I see that adding an ErrorBoundary does keep things from breaking.

Just to confirm:

  1. In the absence of an ErrorBoundary, we expect a failed resource read in <Show /> to permanently break rendering for the whole router.
  2. We expect (1) to behave differently for <Show /> vs <div><Show /></div>.

Is that right?

ryansolid commented 2 months ago
  1. Yes, it's an unhandled/uncaught error we can't predict what will happen.
  2. No I would not expect div wrapper to change this.

It doesn't right? I just added a div and it looks broken still to me.

spiffytech commented 2 months ago

It doesn't right? I just added a div and it looks broken still to me.

Ah, pardon my lack of specificity. When I alter the sandbox to wrap the MyBadComponent's <Show> in a div, MyBadComponent is still broken, but the routing behavior changes from "the whole router is broken permanently" to "only this route broken, but you can still navigate away to other routes".

Altered sandbox

But it sounds like it doesn't really matter either way? Since a <Show> that reads a rejected resource without an ErrorBoundary is undefined behavior?

Would it make sense for Solid to emit a console warning when reading resources without an ErrorBoundary ancestor?

ryansolid commented 2 months ago

Would it make sense for Solid to emit a console warning when reading resources without an ErrorBoundary ancestor?

Maybe.. but it doesn't really know that until you throw an error. We could do an additioanl context lookup at the time of read in dev or something I suppose. It is interesting what sort of ways we could indicate to the developer better patterns.