-
-
Notifications
You must be signed in to change notification settings - Fork 120
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
404 and broken link handling #895
base: main
Are you sure you want to change the base?
Conversation
Otherwise the error boundary itself will throw "In HTML, <h1> cannot be a child of <#document>. This will cause a hydration error." which will result in an infinite re-rendering loop.
If the rsc payload for a link is not available, fall back to a standard http navigation, to trigger host system error handling.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
Hi, thanks for the investigation and the suggestion! I would like to understand the problem better as I assume there should be more minimal solution.
Yeah, it's not surprising because this feature isn't very stable yet. Thanks for the test and the fix! |
I will try my best! Wall of text incoming ... Aside from prefetching (which I agree is an additional feature), all the Waku In our concrete case, the content management system maintains a list of redirects for pages that changed their slug over time. Links to those pages are not necessarily immediately updated in all locations, but just redirect to the targets new slug. That means, a missing RSC payload is an expected case that we need to handle before the actual route change happens. It should not emit an error state to the user, but fall back to the CMS' redirect handler. We are working in a purely static environment, so we can't use a middleware to catch that. Prefetch is part of the solution because it can tell us in advance if a route exists or not, and the And there is the other nice side effect that the Using an error boundary would be more elegant, but it will always unmount the whole page before doing anything else. Maybe this could be handled on a router level, but that means it would apply to every link, which felt even more invasive. |
So, does your solution require prefetching always? It feels bad in two ways: a) prefetch should be optional, b) prefetch shouldn't be awaited. (otherwise, we don't get the benefit of prefetching.) I think we need a solution without prefetching. |
Yes, it always has to prefetch. I tried to trigger the reload right where the exception is thrown: waku/packages/waku/src/client.ts Line 38 in 58d957c
... but this is too late. The error boundary gets rendered already, since the new route is already being evaluated. I think without prefetching, we would have to make the router remember its previous state and render that in case the RSC payload returns a 404. And then trigger the page reload. |
Yeah, I think that's the right solution. It's tedious though. Awaiting prefetch isn't a prefetching any longer, so it's not an option. |
Do you have any pointers where or how to do that? I tried to track the application flow, and from what i can tell the promise returned from waku/packages/waku/src/client.ts Lines 205 to 209 in 58d957c
So the error will always be thrown during rendering, and we are back at the error boundary. Maybe we could store the previous result and and make the error boundary render the whole previous page, but this would still mean a full re-render and probably ui elements re-initializing and flashing. |
I need to sit down and think about it more. As I understand, this is about client side routing, so Meanwhile, would you send a separate PR for 4ca39f7? |
There you go: #896 |
Note Brainstorming different angles to the problem Frontend-redirectsAdd all possible redirects to the js-bundle. ✅ Independent of hosting environment Redirect pagesRender an actual page for each redirect that does a client side redirect on load. ✅ Independent of hosting environment Redirect RSC payloadsWhen there is a redirect from 🛑 Depends on the hosting environment
|
So i had a longer thought about this, and turns out that Option 3 of the brainstorming solves everything:
Nothing in Waku has to change. As an example, I use these Netlify redirects:
When a Adding this rewrite...
... even (almost) solves the 404 problem. When clicking a broken link, the server will respond with the RSC-Payload of the 404 page. It is also correctly displayed, but Waku also replaces the current browser location with |
Thanks for your time thinking about this. Before going into the solution idea, can I make sure I'm on the same page? I'm still not confident if I understand the problem. (like, is it only a hosting issue?)
Can the problem happen with static sites and/or without SSR? |
The problem has shifted through the course of this PR. I'll try to summarize. Initial Situation:Requirements:
Problems:
After the findings from my "brainstorming"
Note The Code in this PR does not reflect its state of discussion any more. I was able to solve everything except that last point outside of Waku. |
Thanks for the summary. That helps my understanding.
404.html is basically only for server routing.
That actually depends on an internal spec that So, I feel like there should be something that waku/router could do. |
Yes, thats a flaw here.
It could handle 404 responses when trying to fetch RSC:
|
The problem
Links that point to non-existent paths will put the router into a faliling state when trying to loading the RSC payload of the target page and throw an exception that is (by default) caught by the global error boundary.
This approach creates some problems for content-focused use cases:
Solution
I changed the Link component to optionally wait for the prefetch to resolve before navigating and if unsuccessful trigger a native navigation to the same path to trigger the host system 404 behavior.
Based on a discussion with @dai-shi , I attempted to achieve the same with a custom error boundary, but the error boundary gets rendered and there is a flash of the error message before the actual navigation happens, which feels like the website is broken.
Note
When writing tests for this, i found that error handling was broken in itself, which is not related to this PR directly: 4ca39f7