solidjs / solid-start

SolidStart, the Solid app framework
https://start.solidjs.com
MIT License
5.08k stars 377 forks source link

[Bug?]: API routes in file based Route Groups don't work in RC0 #1463

Closed timothyallan closed 4 months ago

timothyallan commented 5 months ago

Duplicates

Latest version

Current behavior 😯

having an API route in a route group gives a 404, whereas in previous v.3 it worked. I found this previous issue where it apparently was resolved? https://github.com/solidjs/solid-start/issues/1176

Expected behavior 🤔

I should be able to fire my API from inside a route group.

Steps to reproduce 🕹

to replicate with a new project:

npm init solid@latest

create apiget.tsx with the following :

export function GET() {
  return new Response("Hello World");
}

place it in /routes, and navigate to http://localhost:3000/apiget. Works.

Move apiget.ts x into a new /routes/(blah)/ folder and refresh http://localhost:3000/apiget, and you'll get a 404.

Screenshot 2024-05-01 at 3 29 30 PM

For fun, change apiget.tsx to contain the following:

export default function ApiGet() {
  return <div>Why do I work in a Route Group, but the API call doesn't?</div>;
}

now refresh http://localhost:3000/apiget, and you'll get the component output.

Context 🔦

I have a URL which needs to dynamically redirect to an image after some database checks. That URL is nested in some layout files with nested contexts.

Your environment 🌎

Solid Start rc1, MacOS, Node 20.12.2
timothyallan commented 5 months ago

Just to add that a workaround is to duplicate the routes folder structure for API calls, but not including any () type Route Group folders. It complicates things a lot when you've got a ton of routes, but at least it works.

Edit: Nope that actually doesn't work. The API route then works, but the initial route structure 500's with [h3] [unhandled] TypeError: Cannot read properties of undefined (reading '$GET')

Edit2: What does work is hardcoding matchAPIRoute(path, method) in routes.js to detect when the component path is being tested, and return instead of trying to find a match.

ryansolid commented 5 months ago

Yeah in 0.5 I seperated the API route handling and simplified the matcher. However, I ended up reverting this separation later in the release which is why things don't look different even though they work completely different now. Given that I brought the routes back in I suppose it is fair to see if we can support the other syntax. I do caution in general that co-locating is a poor pattern here as we resolve all API routes before render routes, so things like catch alls won't work perhaps as you might expect when shared in the same scope as page routes. But I see very little reason not to allow them to work under things like route groups given where we landed.

timothyallan commented 5 months ago

Cool thanks Ryan, just to add some more context, the scenario is like this:

For users to view their content, they go to: mydomain.com/viewer/userSlug, to fetch a thumbnail for SEO, the meta tags use mydomain.com/viewer/userSlug/thumb.

The /thumb needs to be a GET API request as it just 301's to a jpg, and the /viewer/userSlug route goes through a bunch of layouts/contexts using Route Groups. With the current setup, /thumb simply doesn't work anymore and 404's.

ryansolid commented 4 months ago

Actually looking at it, it was intended to work. Looks like it was just a small mistake in the regex. Will get this fixed up shortly.

timothyallan commented 4 months ago

Edit, just made a new issue: https://github.com/solidjs/solid-start/issues/1470