RFC: Remove `segment` prop from layouts

[Conduitry]

trafficstars

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 :-)

[lmf-git]

Getting this error for absolutely no reason.

Had to add:

<script>
export let segment;
</script>

Zero reason to need this.

[jazoom]

• client
/src/routes/_layout.svelte
Layout has unused export property 'segment'. If it is for external reference only, please consider using `export const segment`
1: <script>
2:   'use strict';
3:   export let segment;
                ^
4: </script>
5: 
• server
/src/routes/_layout.svelte
Layout has unused export property 'segment'. If it is for external reference only, please consider using `export const segment`
1: <script>
2:   'use strict';
3:   export let segment;
                ^
4: </script>
5: 

So what's the recommended way to stop this being spammed in the logs on every save?

[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]

• client
/src/routes/_layout.svelte
Layout has unused export property 'segment'. If it is for external reference only, please consider using `export const segment`
1: <script>
2:   'use strict';
3:   export let segment;
                ^
4: </script>
5: 
• server
/src/routes/_layout.svelte
Layout has unused export property 'segment'. If it is for external reference only, please consider using `export const segment`
1: <script>
2:   'use strict';
3:   export let segment;
                ^
4: </script>
5: 

So what's the recommended way to stop this being spammed in the logs on every save?

Does it still happen without use strict?

[lmf-git]

I think if you use const you get a different error spammed 😂😂 had same issue

[jazoom]

Does it still happen without use strict?

Yes

I think if you use const you get a different error spammed joyjoy had same issue

Yes, this error:

✗ server
/src/routes/_layout.svelte
Unexpected token
1: <script>
2:   'use strict';
3:   export const segment;
                         ^
4: </script>
5: 
✗ client
/src/routes/_layout.svelte
Unexpected token
1: <script>
2:   'use strict';
3:   export const segment;
                         ^
4: </script>
5: 

But I guess that's because you need to initialise a const at declaration. I don't know why it recommends that as a solution.

[jazoom]

Removing export let segment; entirely leads to this being spammed in the browser console:

<Layout> was created with unknown prop 'segment'

so either way, you get spammed.

[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 https://github.com/sveltejs/sapper/issues/1545#issuecomment-698234671 before people actually start working on this.