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

Implement new layouts system #6174

Merged
merged 47 commits into from
Aug 24, 2022
Merged

Implement new layouts system #6174

merged 47 commits into from
Aug 24, 2022

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented Aug 23, 2022

How to migration from named layouts

Named layouts have been removed in favor of (groups). See the documentation for an introduction: https://kit.svelte.dev/docs/advanced-routing#advanced-layouts

How to migrate highly depends on your use case:

Use case 1: Rewinding to an earlier layout / "breaking out"

If you used named layouts to rewind the layout inheritance chain to a layout higher up the tree, you can reference the segment directly now:

src/routes/
├ marketing/
│ ├ pricing/
│ │ ├ business/
│ │ │ └ +layout@marketing.svelte // reference the "marketing" segment (i.e. the folder with the name) in your routes
│ │ └ +layout.svelte
- ├ +layout-marketing.svelte
- ├ +layout@marketing.svelte
+ └ +layout.svelte

Use case 2: Rewinding to a blank root layout

If you had +layout-blank.svelte in your root to reset to a blank layout, you need to adjust your actual root layout to be blank and move that functionality into another layout further below in the tree, either using an if-statement in your root layout or using the (group) feature:

src/routes/
+ (main)
+ ├ marketing/
+ │ ├ pricing/
+ │ │ └ +layout@.svelte
+ └ +layout.svelte // this is now what used to be src/routes/+layout.svelte
- marketing/
- ├ pricing/
- │ └ +layout@blank.svelte
- +layout-blank.svelte
└ +layout.svelte // this is now what used to be src/routes/+layout-blank.svelte

Use case 3: Different layouts for routes at the same nesting level

If you used named layouts to specify "these 2 pages get layout X and these other 2 next to them get layout Y", use groups instead.

Before:

src/routes/
├ about/
│ └ +page@unauthed.svelte
├ admin/
│ └ +page@authed.svelte
├ home
│ └ +page@unauthed.svelte
├ user/
│ └ +page@authed.svelte
├ +layout-authed.svelte
└ +layout-unauthed.svelte

After

src/routes/
├ (authed)
│ ├ admin/
│ │ └ +page.svelte
│ ├ user/
│ │ └ +page.svelte
│ └ +layout.svelte
├ (unauthed)
│ ├ about/
│ │ └ +page.svelte
│ ├ home
│ │ └ +page.svelte
│ └ +layout.svelte

Use case 4: arbitrary inheritance chains

If you previously used named layouts to enhance other named layouts, which also enhance layouts, to mix and match them, then that's no longer possible this way. In cases like this - which from reading the layout chains probably got confusing anyway - we advice to use composition and/or if-statements instead. The new documentation has a section with an example for this as inspiration.

PR description

#6124

Closes #6196 (using (groups) and/or composition)
Closes #5763 (root layout guaranteed to always exist + await parent())
Closes #5311 (+page@.svelte)
Closes #4940 (no longer possible to get into this situation)
Closes #2154 (only a single root layout now)

  • documentation
  • prevent conflicts
  • print useful error when encountering a +layout-foo.svelte
  • simplify the code that currently has to deal with named layouts
  • update write_types
  • don't reference root layout/error in dictionary in client-manifest.js
  • get it to work in windows

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

@changeset-bot
Copy link

changeset-bot bot commented Aug 23, 2022

🦋 Changeset detected

Latest commit: 7c74aa9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR


> `default` is a reserved name — in other words, you can't have a `+layout-default.svelte` file.
Not all use cases are suited for layout grouping, nor should you feel compelled to use them. It might be that your use case would result in complex `(group)` nesting, or that you don't want to introduce a `(group)` for a single outlier. It's perfectly fine to use other means such as composition (reusable `load` functions or Svelte components) or if-statements to achieve what you want.
Copy link
Member

@dominikg dominikg Aug 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! this is so important to mention and myself overlooked it too, groups are powerful and can have footguns if not handled correctly, but as always, use the right tool for the job.

Should this be in a highlighted box?

And it would be great to elaborate on the reusable layouts or conditionals inside a single layout to establish best practices. Maybe linking to an examples repo?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I enhanced the section with a little code example. For more a repository or an entry in the recipes section on learn.svelte.dev would be good indeed - but we can do that later 😄

Co-authored-by: Conduitry <git@chor.date>
@maximedupre
Copy link

maximedupre commented Aug 24, 2022

Hi 😊

When will this be available? I'm asking because I did a bunch of modifications based on the docs, but now I just realized that the doc has been deployed before the package 🥹

And I don't remember how to do the previous named layout method (and no more doc on this) 😅

@Rich-Harris
Copy link
Member Author

@maximedupre I just merged the release PR (#6217) so it should hit npm any minute now

@tsukhu
Copy link

tsukhu commented Aug 25, 2022

Excellent change love it

@oneezy
Copy link

oneezy commented Aug 27, 2022

This PR broke symlink routes

❌ symlinks break 1.0.0-next.432
✅ symlinks work 1.0.0-next.431

Created issue #6303 with repro


EDIT:
I believe the issue is located somewhere within:
packages/kit/src/core/sync/create_manifest_data/index.js

Doing a little bit of research I found the symlink issue was fixed back in PR #4957

The Fix (commit eb2ab91)

As per @Conduitry's comment, this would error on symlinks to files, as readdirSync throws when not called on a directory.

There seems to be a problem with Node's fs.readdirSync(dir, { withFileTypes: true }) returning false for isDirectory() on symlinks (open issue at nodejs/node#30646). It doesn't seem likely to get fixed upstream in a timely manner, so reverting to fs.statSync instead of withFileTypes seems like a better solution.

Originally posted by @mrkishi in #4957 (review)

EthanThatOneKid added a commit to EthanThatOneKid/acmcsuf.com that referenced this pull request Oct 7, 2022
EthanThatOneKid added a commit to EthanThatOneKid/acmcsuf.com that referenced this pull request Oct 7, 2022
TODO: Reset layout.
Read: <sveltejs/kit#6174>.
EthanThatOneKid added a commit to EthanThatOneKid/acmcsuf.com that referenced this pull request Oct 7, 2022
TODO: Reset layout.
Read: <sveltejs/kit#6174>.
@sifferhans sifferhans mentioned this pull request Dec 16, 2022
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants