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 fallthrough #2440

Closed
lemmon opened this issue Sep 16, 2021 · 7 comments · Fixed by #3217
Closed

Layout fallthrough #2440

lemmon opened this issue Sep 16, 2021 · 7 comments · Fixed by #3217
Labels
feature request New feature or request
Milestone

Comments

@lemmon
Copy link

lemmon commented Sep 16, 2021

Describe the bug

I am trying to throw an error:

export async function load({ page }) {
  if (page.params.foo !== 'okay') {
    return
  }
  return {}
}

This example works as expected within a page component. However when I put this logic inside __layout.svelte it behaves rather oddly.

In first case when nothing is returned then application proceeds to the page component. It renders the page contents, however ignores the __layout content. When the page is refreshed and reloaded completelly it thows an error 500 saying "load must return a value except for page fall through"

I want to check path validity for all the child pages within one directory and putting this logic inside every single page is not ideal. Better solution would to be able to check only once, inside the layout component.

I understand this is probably outside of the scope of intended functionality but it might be a mice addition, in my opinion.

Reproduction

https://github.com/lemmon/svelte-kit-layout-error

Logs

No response

System Info

System:
    OS: macOS 11.5.2
    CPU: (4) x64 Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz
    Memory: 49.27 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 14.17.1 - /usr/local/bin/node
    npm: 7.22.0 - /usr/local/bin/npm
  Browsers:
    Brave Browser: 88.1.19.86
    Chrome: 93.0.4577.82
    Firefox: 92.0
    Safari: 14.1.2
  npmPackages:
    @sveltejs/kit: next => 1.0.0-next.168 
    svelte: ^3.34.0 => 3.42.6

Severity

serious, but I can work around it

Additional Information

No response

@dummdidumm
Copy link
Member

What do you want to achieve / what is your expected behavior when you return nothing from a load function in __layout? AFAIK, fall-through routes exist for pages only, not for layouts.

@lemmon
Copy link
Author

lemmon commented Sep 16, 2021

I want it to behave the same as the page endpoints. If the __layout returns nothing it should fall-through. I understand this issue is part bug and part feature request.

I have this routes structure:
routes/[foo]/ .. and many pages here.

I don't want to check validity of foo inside every single page but only once in the layout.

@Conduitry
Copy link
Member

Conduitry commented Sep 16, 2021

It sounds to me like, if anything should change here, there should be a more explicit error about load not returning anything during client-side navigation to this page as well.

@Conduitry Conduitry reopened this Sep 16, 2021
@dummdidumm
Copy link
Member

I want it to behave the same as the page endpoints. If the __layout returns nothing it should fall-through. I understand this issue is part bug and part feature request.

So you have

routes/[foo]/..
routes/[bar]/..

and you want to fall through to bar by returning nothing from the __layout in foo?

@lemmon
Copy link
Author

lemmon commented Sep 16, 2021

Yeah. Fall through either to [bar] or an __error page. I don't have bar in my case. The very thing is possible when you return nothing form either a page or an endpoint. But it's quite anoying and ugly when you have a lot of pages under the foo (or bar). Seems like __layout is the best place to validate such scenario but the functionality is not there.

@benmccann benmccann changed the title Layout Error Layout fallthrough Sep 17, 2021
@benmccann benmccann added the feature request New feature or request label Sep 17, 2021
@benmccann benmccann added this to the 1.0 milestone Sep 17, 2021
@nhe23
Copy link
Contributor

nhe23 commented Jan 3, 2022

I'd like to work on this issue and will create a PR as soon as I'm done.

@nhe23
Copy link
Contributor

nhe23 commented Jan 3, 2022

Turns out the solution to this issue is pretty easy. All I had to do was to remove a is_leaf check in the load_node function in the server runtime

// if leaf node (i.e. page component) has a load function
// that returns nothing, we fall through to the next one
if (!loaded && is_leaf && !is_error) return;

and in the client-side renderer:

else if (is_leaf && module.load) {
// if the leaf node has a `load` function
// that returns nothing, fall through
return;

However it seems like this behaviour was actively prevented in the past. Was there a reason for that?

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

Successfully merging a pull request may close this issue.

5 participants