Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Add support for nested routes #262

Closed
tony-sull opened this issue May 8, 2018 · 29 comments
Closed

Add support for nested routes #262

tony-sull opened this issue May 8, 2018 · 29 comments

Comments

@tony-sull
Copy link

This one came up in Gitter today and seems worth investigating. It could be really helpful for Sapper to support nested routing, potentially with a special slot, <sapper:child /> or similar.

Sapper currently handles routes by walking the routes directory and finding the matching path (/settings/profile.html or /settings/profile/index.html for example). In cases where you want a shared Settings layout that changes content, you currently need to create something like a SettingsLayout.html component and use that to wrap content in /settings/profile.html. i.e. The child route needs to know all parent components and include them in it's layout.

This gets a little tricky with multiple levels of nested routes. Ember, Angular, and React Router have slightly different solutions for nesting routes - worth investigating how they solved it and what makes the most sense for Svelte/Sapper

@tony-sull
Copy link
Author

@Rich-Harris
Copy link
Member

Been reading the linked docs and, well, hoo boy. I can't believe it's nearly as complicated as Vue, Ember and Angular make it out to be, with special elements and extra configuration all over the place.

React Router has by far the most sensible solution to this problem, though the design of RR is fundamentally at odds with that of Sapper.

I think the solution is two-fold:

  1. Add some way of naming a page that says 'match multiple path parts'
  2. Use #if blocks in the template

By which I mean that we could have a page like routes/settings/[+]/index.html, it would match /settings, /settings/notifications, /settings/notifications/emails and so on.

That file could look like this:

<h1>Settings</h1>

{#if menu[0] === 'notifications'}
  <Notifications {menu}/>
{:else}
  <!-- ... -->
{/if}

<script>
  export default {
    components: {
      Notifications: './_components/Notifications.html'
    },

    preload({ path }) {
      return {
        menu: path.split('/')
      };
    }
  };
</script>

(Note that if we change the default behaviour outlined in #295, whereby the page component persists between navigations if the constructor is unchanged, then we would need to implement something like the persist option discussed therein.)

Am I being naive? Or would that cover the cases we care about?

@silentworks
Copy link
Contributor

Your solution sounds like a lot of manual work, where-as Ember and Vue is not so manual.

@Rich-Harris
Copy link
Member

Huh. Genuinely surprised at that reaction — in Vue we have to add this kind of boilerplate...

const router = new VueRouter({
  routes: [
    { path: '/user/:id', component: User,
      children: [
        {
          // UserProfile will be rendered inside User's <router-view>
          // when /user/:id/profile is matched
          path: 'profile',
          component: UserProfile
        },
        {
          // UserPosts will be rendered inside User's <router-view>
          // when /user/:id/posts is matched
          path: 'posts',
          component: UserPosts
        }
      ]
    }
  ]
})

...and Ember has all this gubbins, which you have to maintain separately from the route templates/controllers themselves, and which involves knowledge of the router API:

Router.map(function() {
  this.route('about');
  this.route('contact');
  this.route('rentals', function() {
    this.route('show');
    this.route('show', { path: '/:rental_id' });
  });
});

Do you mean that the if...else stuff is potentially a lot of manual labour? My thinking is that if it's purely based on props.path then you can tackle that stuff however you want — with if...else, or with <svelte:component this={SubMenu}> (where SubMenu is a computed property), or whatever else you can dream up, all without learning new stuff other than [+].

@TehShrike is typing some interesting ideas into Discord as we speak though, which may simplify things further...

@arxpoetica
Copy link
Member

I like @Rich-Harris's proposal. Question: what happens if one wants to override a particular nesting. In your scenario:

Give that routes/settings/[+]/index.html matches:

  • /settings
  • /settings/notifications
  • and /settings/notifications/emails

What if one creates an actual new nested component index.html file at:

routes/settings/some-new-nesting/index.html

What takes precedence?

@Rich-Harris
Copy link
Member

@arxpoetica routes/settings/some-new-nesting/index.html would take precedence, same as it currently would if you had that alongside routes/settings/[submenu]/index.html

@arxpoetica
Copy link
Member

Just so we capture, this from @TehShrike

I don't recall exactly how nested directories are handled now, so it might be a big breaking change, but how I initially thought Sapper worked (and how I still think would be a good idea) would be for the file /menu/notifications/index.html component to be nested inside the /menu/index.html component when the user visited site.com/menu/notifications. That would only require a slot-like element to tell Sapper where in the menu component the current child should be nested. I suppose it could literally be a named slot.

it would presumably be a named slot with a name that made sense, like <slot name="route"></slot> or something

@tony-sull
Copy link
Author

I had a similar idea as @TehShrike on that one, I'm partial to it being a convention-based solution. There are plenty of scenarios where an app may need to nest routes and have the child route hosted in the parent's container. I think the original solution there was a Layout component, but that got a bit unwieldy if I remember right.

As for how it handles precedence, I'd say something like this for the route /routes/settings/account/index.html

Does the parent route, /routes/settings/index.html in this case, have a slot for sub-routes?
If yes, render the parent while doing the same check for it's parent route, then slot in the sub-route
If no, just render /routes/settings/account/index.html and ignore any parent routes

The only interesting case I can think of would be something like /routes/settings/account.html. Should it check /routes/settings/index.html for a sub-route slot?

@Rich-Harris
Copy link
Member

Now that @TehShrike has opened my eyes, I'm going to try and practice what I preach and think about this from a user perspective rather than an implementer perspective: it would be nice if it worked that way. Among the benefits:

  • Easier to 404 on incorrect routes (/settings/nope wouldn't match if there was no routes/settings/nope.html, and we wouldn't have to have a bunch of logic in preload to get the correct behaviour)
  • We don't have two separate approaches to routing — everything is filesystem-based
  • No need to have complicated ways to decide whether a page should be persisted or not — navigating from /settings/notifications/emails to /settings/notifications/desktop would just replace the emails part, leaving settings and notifications intact
  • Code-splitting can happen at a more granular level without all the complexity involved in userland dynamic imports

Presumably it would look something like this:

<!-- routes/settings/html -->
<h1>Settings</h1>

<Sidebar/>

<div class="submenu">
  <sapper:view foo={bar}/>
</div>

That would be roughly equivalent to this...

<!-- routes/settings/html -->
<h1>Settings</h1>

<Sidebar/>

<div class="submenu">
  <svelte:component this={Submenu} foo={bar}/>
</div>

...except that Submenu would be lazily loaded. (Because everything is statically analyzable, we wouldn't need to wait to load routes/settings.html before loading routes/settings/notifications.html; we could load the two things in parallel.) In all likelihood that's how we'd implement it as well; the main challenge would be figuring out how to inject Submenu into a nested component.

So, what do we reckon? Is this the right approach?

@Rich-Harris
Copy link
Member

@tonyfsullivan missed your post while I was replying! To this point:

The only interesting case I can think of would be something like /routes/settings/account.html. Should it check /routes/settings/index.html for a sub-route slot?

Yes, I think so — elsewhere in Sapper /routes/settings/account.html and /routes/settings/account/index.html are functionally identical.

@Rich-Harris
Copy link
Member

A further thought, not entirely finished — would we need app/App.html if we had this? Or would routes/index.html serve the same need?

@johanalkstal
Copy link

I like the potential simplicity of the idea of /routes/page/index.html being the "Layout" component of subroutes if it has a kind of <childslot></childslot> component in it.

It's elegant.

@johanalkstal
Copy link

johanalkstal commented Jun 27, 2018

@Rich-Harris Is there a way right now to "opt-out" of the App.html component?

Some pages might not require the overall layout of all other pages, for instance log in pages or special landing pages or whatnot.

In that regard, not having App.html would be beneficial and you could decide which "layout" to use for which page with a parent index.html file.

@tony-sull
Copy link
Author

I do think we could get rid of app/App.html with this approach. To @johanalkstal's question, if we scrap App.html then this approach would allow apps to share /routes/index.html when desired but also do things like /routes/dashboard/index.html' and /routes/settings/index.html' where they act as unique wrapper layouts for each module of the app

@paulocoghi
Copy link

paulocoghi commented Jun 27, 2018

What I will say here probably was discussed on other issues here and/or on Svelte repository.

I really like the following approach, that I already develop on top of Svelte, in which we pass the routes and their components.

IMHO, this approach can solve a lot of common issues with routes like: nested routes, reused components on different routes, different routes with the same components, etc.

Do you think this approach might be interesting for Sapper? If so, I will be happy to study Sapper thoroughly to bring my implementation to it (for example, as an option).

Note: I understand that my suggestion is arriving too late, as Sapper has already followed another model to define and organize the app routes and its components. But share your opinions.

Basic case:

app.areas = [ 'content' ];

// This will automatically load the component "components/HelloWorld.html" in the route "/"
app.route[ '/' ] = {
	content: 'HelloWorld'
};

Multiple area and components with reused components

app.areas = [ 'content', 'footer' ];

// On this example the component "components/HelloWorld.html"
// will not be "re-rendered" when switching from the route "/" to "/other"
// and vice-versa.

app.route[ '/' ] = {
	content: 'HelloWorld',
	footer: 'Footer1'
};

app.route[ '/other' ] = {
	content: 'HelloWorld',
	footer: 'Footer2'
};

Nested routes, that I named "route groups"

app.areas = [ 'header', 'content', 'footer' ];

app.group[ '/settings' ] = {
	
	// This is the root route for "/settings"
	'/': {
		header: 'Menu'
		content: 'Settings/Main' //components/Settings/Main.html
		footer: 'Footer'
	
	}

	// This is the route "/settings/notifications"
	'/notifications': {
		header: 'Menu'
		content: 'Settings/Notifications' //components/Settings/Notifications.html
		footer: 'Footer'
	
	}
};

I have different use cases solved with this approach, which if I put here would make the list extensive, but I am available to expose other cases whenever requested.

That's it :)

@paulocoghi
Copy link

paulocoghi commented Jun 27, 2018

With the approach I explained above, the location of the component files doesn't matter.

In my implementation I defined a root folder components, and you can organize them the way you like, independently from where they are used.

@paulocoghi
Copy link

The technical part of my implementation is that, since we have all the app's routes and components, we can programmatically create the code for the "main" app (also with Svelte) that will lazily load the respective components for each route, including all sub-components without having to specify them on the main "map".

@silentworks
Copy link
Contributor

@paulocoghi what you are suggesting is what Absolute State Router does, the only issue with ASR at the moment is that it doesn't support Server Side Rendering. But as soon as that is done, there is no reason why someone wouldn't be able to create their own Sapper like setup with it.

@paulocoghi
Copy link

paulocoghi commented Jun 28, 2018

@silentworks, more than suggesting, its already done. My suggestion is to merge my work with Sapper (if @Rich-Harris understand that this model can be complementary to the existing one on Sapper).

No problem if the conclusion is that these two models are considered very different and there isn't much sense in merging them.

;)

@Rich-Harris
Copy link
Member

@johanalkstal you can't currently opt out of App.html, but you can customise its behaviour however you like:

<!-- App.html -->
{#if props.path.startsWith('/special')}
  <SpecialLayout>
    <svelte:component this={Page} {...props}/>
  </SpecialLayout>
{:else}
  <NormalLayout>
    <svelte:component this={Page} {...props}/>
  </NormalLayout>
{/if}

It seems like we probably could get rid of it if we implement <svelte:view> though. (I'm not sure I like calling it that — better suggestions welcome. <svelte:inject>?)

@paulocoghi I think there's merit in the approach you describe, but I'm afraid I also think it's fundamentally at odds with Sapper's design:

In my implementation I defined a root folder components, and you can organize them the way you like, independently from where they are used.

To me, the whole value of the Next/Nuxt/Sapper approach is that you can't put components wherever you want — there's a clear correct place to put them that means anyone familiar with the framework can instantly understand the codebase. Using the filesystem as a single source of truth is also beneficial from the point of view of code-splitting and generating efficient code (part of the reason Sapper's runtime is so small is that the 'router' is essentially a bunch of regexes in the app/manifest folder).


I think I'm slowly figuring out how all this would work in practice — will see if I can open an RFC on Svelte itself, since a lot of the work would need to happen there.

@arxpoetica
Copy link
Member

<svelte:layout>? <svelte:template>? <svelte:nest>?

@Rich-Harris
Copy link
Member

Ok, so to summarise the chats we've been having in Discord, We Have A Plan.

Rather than introducing a new primitive, the proposal is to make some significant (but straightforward) changes for Sapper 0.15 that will result in no changes to Svelte. Basically, we go from having a flat route structure, where /settings, /settings/notifications and /settings/notifications/email are all possible values of Page, to be applied to the top-level App.html component, to having a hierarchical structure:

// pseudo-code representing what happens in sapper/runtime.js
const [
  { default: App },
  { default: Settings },
  { default: Notifications },
  { default: Email }
] = Promise.all([
  await import('routes/index.html'),
  await import('routes/settings/index.html'),
  await import('routes/settings/notifications/index.html'),
  await import('routes/settings/notifications/email.html')
]);

const [
  data,
  settingsData,
  notificationsData,
  emailData
] = Promise.all([
  App.preload ? await App.preload(...) : {},
  Settings.preload ? await Settings.preload(...) : {},
  Notifications.preload ? await Notifications.preload(...) : {},
  Email.preload ? await Email.preload(...) : {}
]);

const app = new App({
  ...data,
  sapper: {
    child: Settings,
    props: {
      ...settingsData,
      sapper: {
        child: Notifications,
        props: {
          ...notificationsData,
          sapper: {
            child: Email,
            props: emailData
          }
        }
      }
    }
  }
});

Then, your pages could look like this:

<!-- routes/index.html -->
<svelte:component this={sapper.child} {...sapper.props}/>
<!-- routes/settings/index.html -->
<h1>Settings</h1>
<svelte:component this={sapper.child} {...sapper.props}/>
<!-- routes/settings/notifications/index.html -->
<h2>Notifications</h2>
<svelte:component this={sapper.child} {...sapper.props}/>
<!-- routes/settings/notifications/email.html -->
<h3>Email</h3>
<label>
  <input type=checkbox bind:checked=$emailNotifications>
  email notifications
</label>

This would render like so:

<h1>Settings</h1>
<h2>Notifications</h2>
<h3>Email</h3>
<label>
  <input type=checkbox>
  email notifications
</label>

If we then navigated to /settings/notifications/desktop...

<!-- routes/settings/notifications/desktop.html -->
<h3>Desktop</h3>
<label>
  <input type=checkbox bind:checked=$desktopNotifications>
  desktop notifications
</label>

...then this would happen:

const { default: Desktop } = await import('routes/settings/notifications/desktop.html');
const desktopData = await Desktop.preload(...);

const state = app.get();
state.sapper.props.sapper.props.sapper.child = Desktop;
state.sapper.props.sapper.props.sapper.props = desktopData;
app.set(state);

The <h1> and <h2> would be left alone; the <h3> and everything below it would be replaced.

I like this approach because it's nice and explicit. It's consistent, it allows us to do granular code-splitting etc, it lets us use preload at every level of the hierarchy, and it means we can do away with App.html.

Preloading logic

In order to prevent component state from getting corrupted, I think it's important that each level of the hierarchy only has access to parameters that are 'in scope' in preload. So in the case of /products/[category]/[id] (notice that Category.preload is called with the category param, while Id.preload is called with category and id params):

const [
  { default: App },
  { default: Products },
  { default: Category },
  { default: Id }
] = Promise.all([
  await import('routes/index.html'),
  await import('routes/settings/index.html'),
  await import('routes/settings/notifications/index.html'),
  await import('routes/settings/notifications/email.html')
]);

const [
  data,
  productsData,
  categoryData,
  idData
] = Promise.all([
  App.preload ? await App.preload({ params: {}, ... }) : {},
  Products.preload ? await Products.preload({ params: {}, ... }) : {},
  Category.preload ? await Category.preload({ params: { category: 'beer' }, ... }) : {},
  Id.preload ? await Id.preload({ params: { category: 'beer', id: '123' }, ... }) : {}
]);

Navigating to the 'same' page

If it were a case of navigating from /blog/post-one to /blog/post-two — i.e. it's the same routes/blog/[slug].html page in both cases — we would need to set state.sapper.props.sapper.child to null before setting it to Slug so that lifecycle stuff was correctly observed.


Thanks everyone for guiding this process, and for bearing with me while I stumbled towards a design that feels like it could actually work. For the first time I don't have any reservations! If you feel otherwise, speak up, because I'm planning to implement this soon.

tivac added a commit to tivac/svelte-state that referenced this issue Jul 2, 2018
From sveltejs/sapper#262 (comment)

Doesn't actually support any props yet though, not sure how I want to set those up just yet.
@Rich-Harris
Copy link
Member

A further thought: a very common requirement is to indicate which sub-route is selected in the parent route:

<!-- routes/foo/index.html -->
<Sidebar selected={???}/>
<svelte:component this={sapper.child} {...sapper.props}/>
<!-- routes/foo/_components/Sidebar.html -->
<a href="foo/bar" class="{selected === 'bar' ? 'selected' : ''}">bar</a>
<a href="foo/baz" class="{selected === 'baz' ? 'selected' : ''}">baz</a>
<a href="foo/bop" class="{selected === 'bop' ? 'selected' : ''}">bop</a>

It seems like it'd be useful to make that information available in a way that doesn't require the developer to do something complicated involving props.path and computed properties.

Maybe something like this?

<Sidebar selected={sapper.childpathpart}/>

What would be a better name than childpathpart, where it equates to bar, baz or bop in the context of routes/foo/index.html? Actually let me rephrase that, since the answer is 'just about anything': what would be a good name?

@Rich-Harris
Copy link
Member

Apparently the correct term is 'segment', which makes sense: https://english.stackexchange.com/questions/381553/what-could-a-part-of-path-of-url-be-called

So maybe sapper.segment? Or does that imply foo in the context of routes/foo/index.html, rather than bar, baz or bop?

In fact perhaps it'd be more logical to have this...

<!-- routes/foo/index.html -->
<Sidebar selected={child.segment}/>
<svelte:component this={child.component} {...child.props}/>

...though child isn't really viable as a reserved property.

@ekhaled
Copy link

ekhaled commented Jul 3, 2018

segments needs to be an array really.

Imagine hierarchical menu structures.

Each menu in the hierarchy can then just be concerned with it's own level segments[0], segments[1] etc.

Or maybe some other sort of hierarchical structure.

@akaufmann
Copy link

What I don't like so much about <sapper|child>.segment is, that you don't know what the context is if you are new to Sapper. It doesn't tell you that this is a segment of a URI.

How about...
sapper.activeRoutes: {[key: string]: boolean} = {'products': true, 'ratings': true} for e.g. routes/products/[id]/ratings.html
?

@Rich-Harris
Copy link
Member

@ekhaled I think that in most cases you just want the next segment, but if you do need all segments then it can easily be constructed from path, which is available throughout the app:

<ul>
  {#each segments as segment}
    <ol>{segment}</ol>
  {/each}
</ul>

<script>
  export default {
    computed: {
      segments: ({ path }) => path.split('/').filter(Boolean)
    }
  };
</script>

@akaufmann I think that probably holds true whatever we do — it's just something we have to do a good job of documenting. activeRoutes is an interesting idea, though it arguably adds some extra ambiguity (is [id] included, for example?)

The PR is ready (#308), I'm going to merge it and try to get a release out soon, so if anyone has any major objections to the approach we've taken here then now is the time to share them!

@Rich-Harris
Copy link
Member

Alright everyone, I've released Sapper 0.15:

Thanks everyone for shaping this feature. It took a long time for us to get there but I'm extremely pleased with how it turned out. It's now very easy to do things with Sapper that are prohibitively difficult with similar frameworks.

@davidwparker
Copy link

davidwparker commented Aug 1, 2020

In case anyone is looking for the docs- it's here now:
https://sapper.svelte.dev/docs#Layouts

And segment may be going away:
#824

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

No branches or pull requests

9 participants