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

Layout resets #1061

Merged
merged 4 commits into from
Apr 17, 2021
Merged

Layout resets #1061

merged 4 commits into from
Apr 17, 2021

Conversation

Rich-Harris
Copy link
Member

Closes #626.

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

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts

Copy link
Member

@benmccann benmccann left a comment

Choose a reason for hiding this comment

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

going to make a lot of people happy with this one! it was the #2 request from Sapper behind i18n 😄

@@ -62,6 +62,10 @@ We can create a layout that only applies to pages below `/settings` (while inher
<slot></slot>
```

### Resets

To reset the layout stack, create a `$layout.reset.svelte` file. For example, if you want your `/admin/*` pages to _not_ inherit the root layout, create a file called `src/routes/admin/$layout.reset.svelte`.
Copy link
Member

Choose a reason for hiding this comment

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

We should say something about the contents of $layout.reset.svelte. Would it be accurate to say:

To reset the layout stack, create an empty $layout.reset.svelte file.

Copy link
Member

@dummdidumm dummdidumm Apr 17, 2021

Choose a reason for hiding this comment

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

To me this sounds like it needs to be empty, which is not correct because you can start writing your new layout inside the reset file right away. Do I think the current sentence is better.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what would be best but I agree that the current proposed docs are a bit confusing. Maybe it would help if they said one creates the reset file instead of the regular layout file?

Copy link
Member Author

Choose a reason for hiding this comment

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

Made it a bit clearer

@Conduitry
Copy link
Member

Had an API that uses a module export to indicate that this is a reset layout been considered? That seems in a sense tidier than a separate file, especially since they're not allowed to coexist. Was that dismissed because it made it harder to check for at compile time? because it's harder for users to see at a glance?

@Rich-Harris
Copy link
Member Author

Addressed that in #626 (comment). tl;dr the manifest generator (which at present just looks at the names of files on the filesystem, and could theoretically be rewritten to take an array of strings, which is actually probably a good idea anyway) would have to a) know how to preprocess components, which incidentally implies becoming an async function, and b) be sufficiently good at static analysis that it understands all of these, and presumably an infinite number of weird edge cases:

export const reset = true;
const reset = true;
export { reset };
export const reset = some_value();
// export const reset = true;

Otherwise it would have to be determined at runtime by loading layout components higher up the tree, executing load (and hoping that they don't error), then discarding the result of calling load as soon as we encounter a reset.

In any case I think the filename is probably clearer insofar as you can understand the route structure by just looking at the filesystem.

@gudata
Copy link

gudata commented Jan 30, 2023

Is it true that today the way of reseting layout is via the @ at the filename on page/layouts as pointed in docs ?

@JawadAzizi
Copy link

This should be mentioned on the docs page, possibly on this page: https://kit.svelte.dev/docs/advanced-routing#advanced-layouts

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

Successfully merging this pull request may close these issues.

Feature request: Reset layout inheritance in nested routes
7 participants