Skip to content
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

Shared Context for Layouts #5467

Closed
TorstenDittmann opened this issue Jul 11, 2022 · 16 comments
Closed

Shared Context for Layouts #5467

TorstenDittmann opened this issue Jul 11, 2022 · 16 comments

Comments

@TorstenDittmann
Copy link

TorstenDittmann commented Jul 11, 2022

Describe the problem

In certain use-cases you might have some logic (fetching data, state, whatever) inside a __layout.svelte file. If you now want to share behavior with multiple named layouts on the same "level", you are forced to create a component that you can use in each of those or repeat yourself.

I think especially when somebody is working frequently with very diverse layouts on a project, that can be quite repetitive.

Right now the implementation would look like this:

// +layout.svelte
<script>
    import Context from './context.svelte';
</script>

<Context>
    <div class="side-default">
        <slot />
    </div>
</Context>
// +layout-lorem.svelte
<script>
    import Context from './context.svelte';
</script>

<Context>
    <div class="side-lorem">
        <slot />
    </div>
</Context>
// context.svelte
<script>
    // Do something here for all Layouts
</script>

<slot />

Describe the proposed solution

I could imagine something like a +context.svelte, which acts like a layout - but would act like a parent layout to the +layout.svelte and +layout-lorem.svelte files.

Even if a +layout.svelte is not used on the way when using a named layout, the +context.svelte is still applied.

routes/
├─ home/
│  ├─ info/
│  │  ├─ +page.svelte
│  │  ├─ +layout-lorem.svelte // ../+context.svelte still applies here
│  │  ├─ +layout.svelte
│  ├─ +page.svelte
│  ├─ +layout.svelte
│  ├─ +context.svelte
├─ +page.svelte
├─ +layout.svelte
├─ +layout-lorem.svelte
├─ +context.svelte

I am not super confident in the naming here, but I think the idea is that matters.

// +layout.svelte
<div class="side-default">
    <slot />
</div>
// +layout-lorem.svelte
<div class="side-lorem">
    <slot />
</div>
// +context.svelte
<script>
    // Do something here for all Layouts
</script>

<slot />

Alternatives considered

Though it's a very minor inconvenience, I can see room for improvement.

Importance

would make my life easier

Additional Information

I'd be more than willing to dive deeper into the implementation and draft out a RFC if there is interest 👍🏻

@TorstenDittmann TorstenDittmann changed the title Context for Layouts Shared Context for Layouts Jul 11, 2022
@madeleineostoja
Copy link

Pretty sure this was closed by #5778 — sibling layouts can now explicitly inherit from the 'default' layout

  • +layout.svelte — your '_context'
  • +layout-lorem@default.svelte
  • +layout-ipsum@default.svelte

@dummdidumm
Copy link
Member

Thanks for noticing! Closing.

@TorstenDittmann
Copy link
Author

Pretty sure this was closed by #5778 — sibling layouts can now explicitly inherit from the 'default' layout

  • +layout.svelte — your '_context'
  • +layout-lorem@default.svelte
  • +layout-ipsum@default.svelte

@dummdidumm Not sure that it fully solves the problem, only part of it. It still forces me to use the layout of the "default layout" and also doesn't allow me to do things down the line.

This only solves it at a sibling level, but will not for inheritance.

I'll update the initial issue now for the new syntax.

@dummdidumm
Copy link
Member

dummdidumm commented Aug 18, 2022

You can achieve what you want like this:

routes/
├─ home/
│  ├─ info/
│  │  ├─ +page.svelte
│  │  ├─ +layout-lorem.svelte // ../+layout.svelte still applies here
│  │  ├─ +layout.svelte // ../+layout.svelte still applies here
│  ├─ +page.svelte
│  ├─ +layout.svelte // this is like your proposed +context.svelte;  ../+layout.svelte still applies here
├─ +page.svelte
├─ +layout-lorem@default.svelte // this tells +layout-lorem.svelte to reference +layout.svelte
├─ +layout.svelte // this is like your proposed +context.svelte

@TorstenDittmann
Copy link
Author

This will then force me to inherit from the initial layout.svelte, therefore I can only extend it - but not share different logic for when it cannot extend it.

@dummdidumm
Copy link
Member

dummdidumm commented Aug 18, 2022

I thought that's what you want? How would +context make this any different?
If you don't want to extend from the initial layout, you can create another named layout that doesn't inherit the default layout which you reference in your sub layout or page.

routes/
├─ something-else/
│  ├─ about/
│  │  ├─ +page.svelte
│  │  ├─ +layout.svelte // ../+layout@other.svelte still applies here
│  ├─ +layout@other.svelte // extend from the named layout other, not the default layout
├─ home/
│  ├─ info/
│  │  ├─ +page.svelte
│  │  ├─ +layout-lorem.svelte // ../+layout.svelte still applies here
│  │  ├─ +layout.svelte // ../+layout.svelte still applies here
│  ├─ +page.svelte
│  ├─ +layout.svelte // this is like your proposed +context.svelte;  ../+layout.svelte still applies here
├─ +page.svelte
├─ +layout-lorem@default.svelte // this tells +layout-lorem.svelte to reference +layout.svelte
├─ +layout-other.svelte // this one doesn't extend from +layout.svelte
├─ +layout.svelte // this is like your proposed +context.svelte

@TorstenDittmann
Copy link
Author

TorstenDittmann commented Aug 18, 2022

+context would apply to all layouts.

Example from above:

// +layout.svelte
<div class="side-default">
    <slot />
</div>
// +layout-lorem.svelte
<div class="side-lorem">
    <slot />
</div>
// +context.svelte
<script>
    // Do something here for all Layouts
</script>

<slot />

If I would apply your suggested approach, I would end up with:

<div class="side-default">
    <div class="side-lorem">
        <slot />
    </div>
</div>

I don't really have control of what I want to share between layouts

@dummdidumm
Copy link
Member

dummdidumm commented Aug 18, 2022

If you have a default layout and do not reference another layout, that default layout also applies to all other layouts that reference it. So I still don't understand why this shouldn't be possible with named layouts.

From your example:

// +layout-lorem@default.svelte
<div class="side-default">
    <slot />
</div>
// +layout-ipsum@default.svelte
<div class="side-lorem">
    <slot />
</div>
// +layout.svelte
<script>
    // Do something here for all Layouts
</script>

<slot />

And in your sub pages / layouts you either reference one of the named layouts to further inherit from them, or no, in which case you inherit from +layout.svelte, and if you don't want that, create another root layout which you can reference.

@madeleineostoja
Copy link

@TorstenDittmann I think you're getting tangled in the naming, with named layouts your context would become an empty root level +layout with whatever logic you wanted to share. The 'default' layout you would have had before becomes a named layout (layout-root is a common pattern), then it and other layouts can inherit from the 'context' layout, either explicitly if a sibling or automatically if down the tree

@TorstenDittmann
Copy link
Author

TorstenDittmann commented Aug 18, 2022

And in your sub pages / layouts you either reference one of the named layouts to further inherit from them, or no, in which case you inherit from +layout.svelte, and if you don't want that, create another root layout which you can reference.

I understand, but that requires developers to add a named layout to each +page.svelte down the tree until another +layout.svelte is created - because layout-lorem@default.svelte is the intended default in that case.

Also in your example this part doesn't work that way:

routes/
├─ home/
│  ├─ info/
│  │  ├─ +page.svelte
│  │  ├─ +layout-lorem.svelte // ../../+layout-lorem@default.svelte doesn't apply here, even with +page@lorem.svelte
│  │  ├─ +layout.svelte
│  ├─ +page.svelte
│  ├─ +layout.svelte 
├─ +page.svelte
├─ +layout-lorem@default.svelte
├─ +layout.svelte

There is no way for me to make /routes/+layout-lorem@default.svelte apply, without adding another layout to routes/home/+layout-lorem.svelte and repeat what I did before.

Here is a very basic example where I should see 1234 according to the layouts => https://stackblitz.com/edit/sveltejs-kit-template-default-y6poeb?file=src/routes/+page.svelte

I know that there are ways to solve this, but all of them require more steps than necessary in my opinion 👍🏻

@dummdidumm
Copy link
Member

In your example a/+layout.svelte shoud be a/+layout@other.svelte, because you want to reference the other layout in the root. If you don't, you go straight to the root layout, skipping +layout-other@default.svelte.

@TorstenDittmann
Copy link
Author

TorstenDittmann commented Aug 19, 2022

In your example a/+layout.svelte shoud be a/+layout@other.svelte, because you want to reference the other layout in the root. If you don't, you go straight to the root layout, skipping +layout-other@default.svelte.

I know, but that's the point. Also when I do a/+layout@other.svelte I cannot go to the default layout anymore down the road - because other is the default from that point on.

I can't even do a/+layout-other@default.svelte because then again, it would inherit from the default layout and would skip the 2.

In the end I would still be force to use +layout.svelte, +layout-other.svelte and a context.svelte which is used by both, problem is that I have to do it for every level of layout when I am trying to use the other layout down the road.

My proposal is to have a "layout kind file" that is used under any condition, basically forced even when named layouts are used.

@dummdidumm
Copy link
Member

I still don't understand what this makes possible that isn't already possible. If you want to go to the default layout later on, you can do something like

routes/
├─ home/
│  ├─ info/
│  │  ├─ +page.svelte
│  │  ├─ +layout@root.svelte // skip the layout above and go straight to the root layout which extends the default root layout
│  ├─ +page.svelte
│  ├─ +layout@lorem.svelte 
├─ +page.svelte
├─ +layout-lorem@default.svelte
├─ +layout-root@default.svelte
├─ +layout.svelte

@TorstenDittmann
Copy link
Author

I still don't understand what this makes possible that isn't already possible. If you want to go to the default layout later on, you can do something like

routes/
├─ home/
│  ├─ info/
│  │  ├─ +page.svelte
│  │  ├─ +layout@root.svelte // skip the layout above and go straight to the root layout which extends the default root layout
│  ├─ +page.svelte
│  ├─ +layout@lorem.svelte 
├─ +page.svelte
├─ +layout-lorem@default.svelte
├─ +layout-root@default.svelte
├─ +layout.svelte

In the case that home/+layout@lorem.svelte has to initialize state (fetching data into a store for example) that is needed for home/info/.... In your example we skip a layout file.

Let's take an example that is a more real world use-case:

I am working on a project where layout fetches data that is required for all nested layouts that are following. That means that the layouts depend on each other. This basically is a blocker to use named layouts to some extend, since the impact on the added files and addition named layouts added to pages would hurt the readability by a lot.

Bildschirmfoto 2022-08-19 um 15 17 04

The first +layout.svelte after each /[slug]/ fetches data that is needed by a following /[slug]/ to fetch data and so on, because they are in relation.

Now imagine I want to use the /routes/+layout-intro.svelte in /routes/console/[project]/databases/database/[database]/collection/[collection]/document/$create/+page.svelte, I need to rely on the previous layouts to fetch data and therefore they need to be used.

Achieving this with named layouts down the road is not a good solution- I would probably create a lot of dummy layouts just for the sake of allowing the inheritance. I would love to be proven wrong and there is an easy solution, but I also understand that your time is very valuable and my problem might not be that important 🙏🏻

PS: If you prefer I can prepare another Stackblitz example that is more close to my use case, where it might be more obvious.

@dummdidumm
Copy link
Member

To clarify I understand this correctly: You want to reuse the data loading logic from the layouts, but you don't want to reuse their UI?

@TorstenDittmann
Copy link
Author

To clarify I understand this correctly: You want to reuse the data loading logic from the layouts, but you don't want to reuse their UI?

Yes and no, data loading is only one part of it. It is also used to populate breadcrumbs, set title, etc.

In short you could say I only care about the <script /> part. I haven't looked into +layout.js yet, but from a first glance it might not be the ideal fit 🙂

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

No branches or pull requests

4 participants