sveltejs / kit

web development, streamlined
https://kit.svelte.dev
MIT License
18.52k stars 1.91k forks source link

pages using a named layout cause an `GET method not allowed` error when an endpoint with the same name (but no layout suffix) is present in the same folder #5252

Closed UltraCakeBakery closed 2 years ago

UltraCakeBakery commented 2 years ago

Describe the bug

When you've got an endpoint named foo/index.js and a page using a named layout like foo/index@foo.svelte, svelte-kit will throw an GET method not allowed error when trying to visit /foo.

Note that the endpoint does not export a GET handler, and there is no index.svelte.

Reproduction

https://github.com/UltraCakeBakery/svelte-bug-named-layouts-and-endpoints

Logs

No response

System Info

System:
    OS: Windows 10 10.0.22000
    CPU: (12) x64 Intel(R) Core(TM) i7-10750H CPU @ 2.60GHz
    Memory: 3.47 GB / 15.75 GB
  Binaries:
    Node: 16.15.1 - C:\Program Files\nodejs\node.EXE
    npm: 8.5.2 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.22000.120.0), Chromium (102.0.1245.44)
    Internet Explorer: 11.0.22000.120
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.51
    @sveltejs/kit: next => 1.0.0-next.352
    svelte: ^3.46.0 => 3.48.0

Severity

annoyance

Additional Information

You can work around this bug by renaming the index.svelte to index@foo.svelte.

UltraCakeBakery commented 2 years ago

https://kit.svelte.dev/docs/layouts#named-layouts-inheritance-chains

There also seems to be a typo here (I think). It should be x/y/__layout-root.svelte, not x/y/__layout@root.svelte

dummdidumm commented 2 years ago

I'd say this is working as expected. The page endpoint needs to have the same name as the page. I'm not sure if adding behavior like "if the name does match up to the @ sign use that instead would be a good solution. It could throw an error with a better message though, since this seems to come up from time to time

philip-weber commented 2 years ago

To make this a bit clearer. If your route file (.svelte) is using a named layout (eg. index@foo.svelte) then your endpoint also needs the named layout in the file name (eg. index@foo.js or index@foo.ts). The same is true for matchers (as far as I know).

UltraCakeBakery commented 2 years ago

I'd say this is working as expected. The page endpoint needs to have the same name as the page. I'm not sure if adding behavior like "if the name does match up to the @ sign use that instead would be a good solution. It could throw an error with a better message though, since this seems to come up from time to time

It is not working as I expected. The index.jsis technically a regular endpoint. People should be able to visit the index@foo.svelte and make other kinds of request (POST, PUT) to the index.js.

I'm not saying that this is an ideal usage of svelte-kit. Users should probably keep consistent while naming files in the routes, or chose a different routes architecture. I do think this is technically a bug and not working as intended though.

gtm-nayan commented 2 years ago

The index.js is technically a regular endpoint

You're right on that, and according to the sorting rules(the second one) it's working as it should. You make a GET request to "/foo" it matches the "foo/index.js" endpoint, "foo/index.js" doesn't have a GET handler, kaboom.

People should be able to visit the index@foo.svelte and make other kinds of request (POST, PUT) to the index.js.

It looks like what you're really asking for is to bring back fallthrough routes, at least for the presence/absence of handlers instead of for status codes.

UltraCakeBakery commented 2 years ago

The index.js is technically a regular endpoint

You're right on that, and according to the sorting rules(the second one) it's working as it should. You make a GET request to "/foo" it matches the "foo/index.js" endpoint, "foo/index.js" doesn't have a GET handler, kaboom.

People should be able to visit the index@foo.svelte and make other kinds of request (POST, PUT) to the index.js.

It looks like what you're really asking for is to bring back fallthrough routes, at least for the presence/absence of handlers instead of for status codes.

I forgot svelte-kit removed fallthrough routes..

Though the behavior makes more sense to me now, I can imagine it takes someone else a couple hours to figure out what went wrong. I was luckly that I was swapping out my own named layout system with the one from svelte-kit to notice this behavior had something to do with the names of the files.

I think an additional sorting rule would be a good change here, or a different error as it states that I'm not even allowed to add an additional GET handler to the endpoint. GET /foo is now impossible to handle or fetch.

But maybe as a user I should not (be able to) have an index.js when there is a page component with the same name (but prefixed) like index@foo.svelte, for the sake of consistency in the routes folder file structure

UltraCakeBakery commented 2 years ago

While looking for topics about the idea of layout having shadow endpoints, I came across this issue https://github.com/sveltejs/kit/issues/4666 which appears to be talking about the same thing. This issue got marked as a 1.0 by Rich Harris, but closed by Conduitry (i assume) because it was marked a duplicate. Figured I'll would reference it here while i'm at it.

If anyone got a link to the discussion about layouts also having shadow endpoints, please hit me up with it :)

dummdidumm commented 2 years ago

Closing as obsolete

foo/
  | +page@named.svelte
  | +page.server.js

works