unjs / nitro

Next Generation Server Toolkit. Create web servers with everything you need and deploy them wherever you prefer.
https://nitro.unjs.io
MIT License
5.6k stars 482 forks source link

Different wildcard variables in same api folder crash routes #1093

Open issue-up[bot] opened 1 year ago

issue-up[bot] commented 1 year ago
   Forwarded from downstream issue: - https://github.com/nuxt/nuxt/issues/19965 by @tamutus

### Environment - Operating System: `Windows_NT` - Node Version: `v18.15.0` - Nuxt Version: `3.3.2` - Nitro Version: `2.3.2` - Package Manager: `npm@9.6.2` - Builder: `vite` - User Config: `-` - Runtime Modules: `-` - Build Modules: `-` ### Reproduction [Minimum reproduction](https://github.com/tamutus/Nuxt-3-routes-with-different-slug-names): any nuxt project, create files `/server/api/[a].get.ts` and `/server/api/[b].post.ts` and neither route will work, even though they don't semantically conflict. ### Describe the bug If you use different variables for the different methods of a route, none of those routes will work. I don't see why this shouldn't be allowed, but if it's a struggle to fix, it should at least be mentioned in the documentation that slug names have to be identical in a given route, and error messages should reflect this at compile time. I ran into this problem when making mutative API routes require id, while the get route uses a wildcard in a containing folder (i.e. `/[bucket]/file/[title].get.ts` ) and the title to uniquely fetch something. ### Additional context _No response_ ### Logs ```shell-script Console error: `Failed to load resource: the server responded with a status of 405 (Method get is not allowed on this route.)` Response (trimmed directory names but they start with `C:/` ): { url: "/api/a", statusCode: 405, statusMessage: "Method get is not allowed on this route.", message: "Method get is not allowed on this route.", stack: "
at createError .../node_modules/h3/dist/index.mjs:128:15)
at Object.handler (/C:/.../node_modules/h3/dist/index.mjs:1424:13)
at Object.handler (/C:/...node_modules/h3/dist/index.mjs:1247:31)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
at async Server.toNodeHandle (/C:/.../node_modules/h3/dist/index.mjs:1322:7)
" } ``` ```
qin-guan commented 10 months ago

I can confirm this bug exists and is also downstreamed into Nuxt routing, caused quite a bit of confusion.

Would Nitro team consider this expected behavior / planning on a fix?

pi0 commented 10 months ago

This sounds like a bug (seems in scanner). Haven't tried to confirm but PR welcome to fix.

qin-guan commented 10 months ago

Hey @pi0, thanks for the quick reply! I've just checked the scanner (here) but it seems to generate the routes correctly, unless you are referring to another scanner in the codebase 😅 Happy to help investigate more!

qin-guan commented 10 months ago

After further investigation I believe it is actually a bug in h3.

Tested using following code:

import { createServer } from "node:http";
import { toNodeListener, createApp, eventHandler, createRouter } from "h3";

const app = createApp();

const router = createRouter()
  .get(
    "/",
    eventHandler(() => "Hello World!"),
  )
  .get(
    "/hello/:name",
    eventHandler((event) => `Hello ${event.context.params.name}!`),
  )
  .post(
    "/hello/:otherName",
    eventHandler((event) => `Hello ${event.context.params.otherName}!`),
  );

app.use(router);

createServer(toNodeListener(app)).listen(process.env.PORT || 3000);
➜  ~ curl http://localhost:3000/
Hello World!%
➜  ~ curl http://localhost:3000/hello
{
  "statusCode": 404,
  "statusMessage": "Cannot find any path matching /hello.",
  "stack": []
}%
➜  ~ curl http://localhost:3000/hello/hello
{
  "statusCode": 404,
  "statusMessage": "Cannot find any path matching /hello/hello.",
  "stack": []
}%
➜  ~

But it works after removing the post route.

qin-guan commented 10 months ago

I will create an issue in h3 repo to track and we can move discussion to there instead

Hebilicious commented 10 months ago

If I'm not mistaken, https://github.com/unjs/radix3/pull/59 will be a step in the right direction to fix this.