RFC: Remove segment prop from layouts

[Conduitry]

With the introduction of the page store in Svelte v3 Sapper, the segment prop passed to _layout.svelte components feels somewhat vestigial. I don't think there's anything that can be done with it that can't be done (in a more uniform, if slightly more verbose, way) with $page.path.

There seems to be a bit of confusion around segment, about how to gain access to other parts of the URL, and about how to inject other bits of data into the layout. These would hopefully all be avoided if we never actually passed any props into _layout and we instead had the docs tell people to just always use the stores.

Thoughts?

[Conduitry]

We could perhaps ease people into this a bit, by issuing warnings rather than immediately breaking. It should be pretty straightforward to check at build time whether a given _layout component defines a segment prop, by inspecting the vars array returned by the compiler. We're already doing this elsewhere to check for the presence of preload.

[pngwn]

As I mentioned, I have moved away from using segment at all. I find it easier to work with the page store which covers all segment usecases and many more besides. Layout's are pretty static so working out which part of the path they correspond to is pretty straightforward.

Removing segment would make the API more consistent, would be easier to teach and, as you said, remove the question of 'can other props be passed to layouts'. While the change might be slightly controversial, I think in the long term it will make for a clearer more consistent API.

If we do choose to adopt this proposal, transitioning to it gently with a deprecation notice seems like a nice idea, especially since the segment prop is probably quite widely used.

[Rich-Harris]

I do find it useful for e.g. controlling which nav button is selected (including nested, secondary navs). If we were to remove this — and I agree, it's weird and vestigial — what about adding a $page.segments array in its place? Or is it better for the user to derive that from $page.path?

[pngwn]

I wondered about adding a segments array to the page store, it would obviously be useful but its also incredibly straight-forward to derive from the path. Maybe if you needed to do it extensively across an app it could get tedious but in my mind the segments are really just used for navigation or navigation related pieces of UI. I don't know if people use them much outside of the odd navigation component.

[njbotkin]

That's funny, I just started using segment as a clean way to ask, "are there any sub routes mounted to this _layout?" In segment's stead, I might suggest current; the _layout's path. All child segments can be had with $page.path.slice(current.length).

Ideally exposing $page would be a bit less laborious, but I imagine there's a discussion on that elsewhere.

[Conduitry]

$page is already exposed. These stores can be used in every component, including layouts,

[njbotkin]

Sorry, I meant exposing the reference to the page store. It feels a bit icky to have to const { page } = stores() each time. To be clear, I have no better ideas.

[pngwn]

I don't know what "icky" means. I think it is explicit and i like that.

[njbotkin]

It's entirely subjective and appears to be derailing the discussion, so let's move on, I guess 😅

[swyxio]

i like this idea. i'd love to also link this RFC in the docs to warn users. is this kind of PR welcome?

[intrikate]

One thing I believe can’t be done with $page.path alone is error route differentiation, mentioned in #948 (correct me if I’m wrong, but we’ll have to resort to hacks like let isError = segment === undefined && $page.path !== "/" to do that). Other than that, they do seem like one too many, and $page wins everywhere.

[rchrdnsh]

Any updates on this? Having much difficulty wrapping my head around using segment and making it work properly. It works well enough for the use case in the starter:

<li>
  <a
    class={segment === 'about' ? 'selected' : ''}
    href='about'
  >
    About
  </a>
</li>

..but when i try to do the same thing with dynamic routes from slugs from markdown files, then no dice:

<li>
  <a
    class={segment === 'team/{member.slug}' ? 'selected' : ''}
    href='team/{member.slug}'
  >
    {member.name}
  </a>
</li>

...which is a bit frustrating. Pretty confident I'm doing something wrong here, but discord has up to this point been no help.

Also, getting odd warnings in the console without doing anything and just running the starter locally:

...would segment not already be defined by sapper? Quite confusing. Anyway, very curious to know where all of this is at currently and if it will be addressed or has been addressed or if there is a new path forward anytime soon :-)

[rodoch]

Previously I was very much in favour of getting rid of segment, but the more I build more complicated apps with nested layouts the more I appreciate it as an elegant little piece of shorthand. Achieving the same result with the $page store results in a lot more ceremony/boilerplate code. Maybe I'm too late in pointing this out but I thought it was worth voicing another opinion! It will potentially be a breaking change for a lot of people in a lot of places in their apps.

[benmccann]

@rodoch do you have any code you can share that demonstrates the complication you're describing?

[lmf-git]

Sorry that your issue is deemed off-topic lol, it's a major problem that all people will run into. Anyway, best way I can find is to let it give you segment... then just use it as a body class.

It is complaining you aren't using "segment"... so just swallow their opinionated "off-topic" marking bs and use it as a body class.

This will get marked as off-topic so another 10,000 developers have to waste their time on it I imagine. Such is life.

[lmf-git]

Either stop marking comments as off-topic or fix the issue. You're wasting peoples' time that they use to feed their families.

[jazoom]

This will get marked as off-topic so another 10,000 developers have to waste their time on it I imagine. Such is life.

I don't know whether it's off-topic or not since I'm not yet familiar with Svelte/Sapper. I've been using Vue for several years, and React for several years before that. I find it strange that such an issue crops up with something that is basically the official Sapper starter template. On searching, this is the issue that seemed closest to what I'm looking at.

But I certainly agree the behaviour of marking off-topic without any explanation or direction to where one should be looking is pretty useless.

[lmf-git]

@jazoom "use it as a body class" -> Put that in your pipe and smoke it

[rodoch]

@rodoch do you have any code you can share that demonstrates the complication you're describing?

Nothing in a public repo, but I can demonstrate with some pseudo-code. For example, I have a site with several different sections.

index.svelte
- section1
   - _layout.svelte
   - [slug].svelte
   - index.svelte
- section2
   - _layout.svelte
   - [slug].svelte
   - index.svelte
- section3
   - _layout.svelte
   - [slug].svelte
   - index.svelte
- section4
   - _layout.svelte
   - [slug].svelte
   - index.svelte

There is a main site navigation and then each section has it's own subnavigation. The URL structure looks like:

• mysite.com/section1/
• mysite.com/section1/page2/

The subnavigation for each section is different and is contained in each section's _layout.svelte file. In each section we use the segment prop to mark the active page, e.g.

<script>
  export let segment;
</script>

<ul>
  <li>
    <a href='./page1' class:active={segment==='page1'}>Page 1</a>
  </li>
  <li>
    <a href='./page2' class:active={segment==='page2'}>Page 2</a>
  </li>
  <li>
    <a href='./page3' class:active={segment==='page3'}>Page 3</a>
  </li>
  etc. ...
</ul>

If the segment prop was no longer available, we'd have to roll our own function for getting the appropriate page parameter. Not a big deal, says you:

import { stores } from '@sapper/app';

const { page } = stores();

function getParam(paramIndex) {
  if (!$page.path || $page.path === '/') return ``;
  let params = $page.path.split('/');
  return params[paramIndex];
  // + more guards, etc. ...
}

However, because the page store can't be accessed outside of a component it's harder to factor this into a reusable function, and so you could end up writing it in 8 different places in your app (the boilerplate, and not very Svelte). Or else, you write a reusable function with an ugly signature like getParam(page, paramIndex). On the other hand, segment just works™.

This isn't a deal-breaker, so if you'd rather see it gone that's fine. But if it's not doing any particular harm and it's not standing in the way of any architectural progress it would be nice to see it stay. I can see the the unused prop warning, though harmless, is evidently annoying some people in this thread but perhaps there would be some way to quash this?

[dansvel]

hmm,,
may be if you don't need the prop, you can do this?

on _layout.svelte use this two line code

export let segment;
segment = !segment;

[jazoom]

hmm,,
may be if you don't need the prop, you can do this?

on _layout.svelte use this two line code

export let segment;
segment = !segment;

That seems to be all it wants. You don't even need to negate it. This little hack is enough to get it to stop giving warnings.

export let segment;
segment = segment;

[dansvel]

hmm,,
may be if you don't need the prop, you can do this?
on _layout.svelte use this two line code

export let segment;
segment = !segment;

That seems to be all it wants. You don't even need to negate it. This little hack is enough to get it to stop giving warnings.

export let segment;
segment = segment;

oh yeah, haha,, why i even do that,,

edit: i remember doing that so it will not create a warning by the IDE

[jacob-8]

I also use segment over 30 times in production (two different apps) and would be sad to lose it.

[UltraCakeBakery]

Just in case I'll reference to my comment #1545 (comment) before people actually start working on this.

[rodoch]

Just to back up my statement above a little further, you can look at a production app made using Sapper here: https://www.gaois.ie/.

As well as the main navigation, segment gets used within 7 nested layouts in this application to mark the current page within subnavigation menus. If segment was removed, as far as I understand it, I'd have to reproduce this logic 7 places in the application - this doesn't seem to tally with Svelte's "write less, do more" philosophy.

I accept that there are many applications that don't need any kind of subnavigation or nested layouts but the more people build larger, fully-featured sites with Sapper the more this use case will crop up IMO.

[benmccann]

The code doesn't seem like it's much different if you were to use the page store instead of segment:

<ul>
  <li>
    <a href='./page1' class:active={page.path.endsWith('/page1')}>Page 1</a>
  </li>
  <li>
    <a href='./page2' class:active={page.path.endsWith('/page2')}>Page 2</a>
  </li>
  <li>
    <a href='./page3' class:active={page.path.endsWith('/page3')'}>Page 3</a>
  </li>
  etc. ...
</ul>

[rodoch]

Fair point, and maybe my example is at fault there. Both my pseudo-code and your suggestion presume that segment will always map to a page. Here's a real-world example:

https://www.gaois.ie/en/technology/developers/login/

The sub-menu on the left here uses segment to mark the active menu item for ('developers') in at least nine possible URLs, e.g.

https://www.gaois.ie/en/technology/developers/
https://www.gaois.ie/en/technology/developers/in/account/
https://www.gaois.ie/en/technology/developers/in/password/
https://www.gaois.ie/en/technology/developers/in/keys/ etc.

The solution above would not work in this situation - you can't rely on endsWith() - whereas segment solves the problem neatly. (Again, maybe I should have given a less trivial example).

[UltraCakeBakery]

Can't we just leave segment as is but just change the docs? That would make everyone happy?

[lmf-git]

Can't we just leave segment as is but just change the docs? That would make everyone happy?

Nothing will ever make me happy.

[daxxog]

I recently upgraded sapper (0.27.0 to 0.28.10) in my project and came across this same problem that I see someone else was having in this issue -->

I removed nav.svelte
this meant i didnt need the segment in layout anymore.

That causes this console error message:
<Layout> was created with unknown prop 'segment'

When I add the property back to _layout.svelte that console error goes away but I now get a compilation warning.
Layout has unused export property 'segment'. If it is for external reference only, please consider using export const segment

If i switch to a const the unknown prop error shows up again.

I dont think this the error or warning cause issues, but it would be good to fix one or the other.

I am in favor of removing segment, if it helps me have a more concise way to get rid of these warnings. As an alternative, maybe sapper could provide a utility library for accessing segments of the URL navigation / routing so we don't have to write as much $page related boilerplate? I noticed this issue earlier and started writing some util code in my project for $page just in case segment was going away. If anyone thinks this would be a good replacement for segment, maybe I could put together a pull request adding some $page data access utilities.

EDIT:
Something like this -->

import { pageSegment } from '@sapper/app';
const segment = pageSegment();

I think you could maintain some level of backwards compatibility with this method.