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

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

Open
spiffytech opened this issue Aug 30, 2024 · 5 comments

Comments

@spiffytech
Copy link
Contributor

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

  • OS: Linux
  • Browser: Vivaldi, Firefox
  • Version: Vivaldi 6.8.3381.57

Additional context

No response

@ryansolid
Copy link
Member

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

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

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

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

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.

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

No branches or pull requests

2 participants