kit icon indicating copy to clipboard operation
kit copied to clipboard

Support suffixes for [...rest] endpoints

Open ngalaiko opened this issue 3 years ago • 10 comments

Describe the problem

I have a project with a following pages routes structure:

/routes/index.svelte
/routes/posts/index.svelte
/routes/posts/index.json.js
/routes/posts/category1/index.svelte
/routes/posts/category2/index.svelte
/routes/posts/[...catchall]/index.json.js
/routes/posts/[...catchall]/index.svelte

What I am doing there:

  • /routes/posts/index.{svelte,json.js} is a list page with every post
  • /routes/posts/category*/**/*.svelte are posts with just markup and some props
  • /routes/posts/[...catchall]/index.svelte fires when no post if found, so it queries /routes/posts/[...catchall]/index.json.js to check if the post has an alias and redirects

What I would like to do is to add a generic /likes.json endpoint to all the posts, so:

...
/routes/posts/[...catchall]/likes.json.js
/routes/posts/[...catchall]/index.svelte
/routes/posts/[...catchall]/index.svelte

But the problem I am facing is that the request is routed to the /routes/posts/[...catchall]/index.svelte handler, even though according to sorting rules, likes.json must have a priority

Describe the proposed solution

Support catchall suffixes

Alternatives considered

I could solve this without having [...rest] matcher of course, but that involves either copy-paste, or manual routing.

Importance

would make my life easier

Additional Information

This seems like an improvement of an existing feature

ngalaiko avatar Apr 19 '22 12:04 ngalaiko

There is a bug here — because the client-side routing table doesn't include endpoints, the router doesn't 'see' that /posts/[...catchall]/likes.json.js is a better match for /posts/a/b/c/likes.json than /posts/[...catchall]/index.svelte. We might have to include endpoints in the routing table to fix that.

That said, how are you running into this bug? If you hit /posts/a/b/c/likes.json directly, or fetch it, the data will come directly from the server. If you're linking to the JSON file for some reason, you can workaround the bug by putting sveltekit:reload on the link. Is there some other way you're being sent to the wrong route?

Rich-Harris avatar Apr 20 '22 14:04 Rich-Harris

We had previously (#2656) decided to not tell the client-side router about endpoints, and had said if you wanted to link to them, you'd need sveltekit:reload. If we're sticking with that, this doesn't sound like a bug I don't think. Are you considering revisiting that now that we've overhauled routing a bit and eliminated route fallthrough?

Conduitry avatar Apr 20 '22 14:04 Conduitry

No, I just forgot we'd made that call. Now that we've eliminated fallthrough, it's possible that we could reinstate the previous behaviour in a less costly way — maybe it's possible to only tell the client-side router about ambiguous cases like this one, rather than all endpoints.

Rich-Harris avatar Apr 20 '22 18:04 Rich-Harris

Having the client-side router know about endpoints (and use a full-page navigation in that case) only when there's an ambiguity - but to have it continue to show the client-rendered 404 page when linking to any other endpoint - sounds confusing to me. That's worse than either consistently having it know about endpoints or consistently having it not know about endpoints.

Conduitry avatar Apr 20 '22 19:04 Conduitry

Unmatched routes already trigger a full page navigation by default; the client only renders 404s for matched page routes, hence the bug with ambiguous routes.

mrkishi avatar Apr 20 '22 20:04 mrkishi

Oh! I had forgotten about that change. Yup, makes sense to just include this for potentially ambiguous endpoints.

Conduitry avatar Apr 20 '22 20:04 Conduitry

Thanks for looking into it.

Here is a reproduction sandbox: https://stackblitz.com/edit/sveltejs-kit-template-default-pkjzkn?file=src/routes/index.svelte

If you hit /posts/a/b/c/likes.json directly, or fetch it, the data will come directly from the server. If you're linking to the JSON file for some reason, you can workaround the bug by putting sveltekit:reload on the link. Is there some other way you're being sent to the wrong route?

That actually doesn't seem to be the case, that's why I opened it.

I want to be able to both query the endpoint myself with let's say curl, and also use it inside a load() function to pretender data in svelte kit

ngalaiko avatar Apr 21 '22 07:04 ngalaiko

Ah okay, that seems to be a completely different bug — index.json.js is being sorted before likes.json.js in the server-side routing table.

Rich-Harris avatar Apr 21 '22 13:04 Rich-Harris

Found the issue: https://github.com/sveltejs/kit/blob/062f38e9c1716089d3998da9110627c48190ed11/packages/kit/src/core/sync/create_manifest_data/index.js#L386-L388 This comparison function does the sorting of values.

a.segments: [[{"...stuff",dynamic},{".json",not-dynamic]]
a.id: [...stuff].json

b.segments: [[{"...stuff",dynamic}][{"specific.json",not-dynamic]]
b.id [...stuff]/specific.json

Since The first argument has more parts in the first segment, it wins.

Can be fixed by adjusting compare(a,b) or id, segments logic

elikoga avatar Jul 13 '22 20:07 elikoga

This is impacted by #5748 and could possibly be closed by that?

elikoga avatar Aug 02 '22 13:08 elikoga

indeed it can :)

Rich-Harris avatar Aug 16 '22 15:08 Rich-Harris