EmilTholin / svelte-routing

A declarative Svelte routing library with SSR support
MIT License
2.02k stars 178 forks source link

Nested routes syntax means you can't have a single top-level 404 handler #134

Open kevinrenskers opened 4 years ago

kevinrenskers commented 4 years ago

When you take the example project in this repo, App.svelte says that everything under blog/* is a valid url, to be handled by the Blog component. The Blog component only accepts first, second and third as valid sub-URLs. What if you go to /blog/fourth? This needs to be "caught" inside the Blog component with a pathless <Route>, it can't be handled in the main App.svelte file.

That means that if you have multiple "top level routes" that all accept /* to be handled in nested routers, you now have to add 404 handling to all these nested routers. Worse, if your nested routers actually show nested content, this means that your 404 component is now potentially shown inside of (multiple) parent layouts.

It's too easy to forget to add 404 handling to a subrouter, because you expect the top level router to catch these URLs. And showing the 404 page inside a nested layout is often not great either.

As an example, when I go to /campaigns/abc/locations/nonexistingid I can only show the 404 component inside my nested layout, since the `/campaigns/abc/locations is valid and handled by 2 levels of routers.

Screen Shot 2020-04-19 at 01 28 35

And again, it needs to be handled like this inside every nested router.

Worse, it makes it impossible to catch some invalid routes due to the need to use /* in the parent router. For example inside my location detail page I have a route for the edit modal.

<Router>
  <h1>{location.name}</h1>
  <a href="/locations/{location.id}/edit">edit</a>

  <Route path="edit">
    <EditModal on:close={() => navigate(`/locations/${location.id}`)} {location} />
  </Route>
</Router>

But for this to work, the parent router needs to be like this:

<Router>
  <Route path="{location.id}/*"><Location {location} /></Route>
</Router>

But this means that if I go to /campaigns/abc/locations/xyz/lalalala, there is no 404 shown anywhere, it just shows the location detail page as if nothing happened. Maybe not a huge problem in this particular instance, but you shouldn't be able to type in random stuff behind a url and it just keeps on working.

kevinrenskers commented 4 years ago

I tried moving the edit route to the top level router like this:

<Router>
  <Route path="{location.id}/edit">
    <Location {location} />
    <EditModal on:close={() => navigate(`/locations/${location.id}`)} {location} />
  </Route>
  <Route path="{location.id}"><Location {location} /></Route>
</Router>

That would solve the need of having to have that /* catch-all. But, this causes the Location component to be shown twice on screen while the modal disappears with an animation. (It works if I remove all animations from showing and hiding the modal.)

Screen Shot 2020-04-19 at 01 55 42

All in all, a lot of problems that I'm running into :/ That said, I really do like this project, well done!

robomatic commented 4 years ago

I'm working on nested routes as well. I can route correctly but I get no catchall to 404, instead it refreshes the page. App.svelte

<Router path="*" component={404Page} />
<Router path="settings/*" component={Settings} />

then I set my nested routes Settings.svelte

<Router path="profile" component={SettingsProfile} />
...
robomatic commented 4 years ago

Well after some testing I see that catchall route works when navigated using programmatic or Link component navigation.

robomatic commented 4 years ago

Some further issues. Seems from within nested routers default routes the path must include the root route then nest route. <Route><button on:click={()=> navigate('root/nest1')}>button</button></Route> However from within named nested route you can leave that root route out. <Route path="nest1"><button on:click={()=> navigate('nest2')}>button</button></Route>

kevinrenskers commented 4 years ago

https://github.com/AlexxNB/tinro has fallback handling that bubbles up to the nearest parent route that has a handler. It's missing some other important things though.

EmilTholin commented 4 years ago

Hi @kevinrenskers!

Yes, this is very unfortunate problem that we have no satisfying solution to at the moment. It is complicated further by that this needs to work synchronously in one pass on the server as well, so having fallback handling that bubbles up to the nearest parent route is (to my current understanding) impossible.

I will look into this more seriously when I have the time and see if there is something that can be done about it.