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

route files with - in param name are not scanned #2868

Closed
moshetanzer opened this issue Nov 11, 2024 · 8 comments · Fixed by #2872
Closed

route files with - in param name are not scanned #2868

moshetanzer opened this issue Nov 11, 2024 · 8 comments · Fixed by #2872
Labels
bug Something isn't working

Comments

@moshetanzer
Copy link
Contributor

moshetanzer commented Nov 11, 2024

Environment

latest Nuxt 3.14.159
node 23.1

Reproduction

https://github.com/moshetanzer/nitro-server-route-bug

Describe the bug

Hi,

Hope you are well.

I came across the above bug - not sure if it done like this on purpose.

A route with a dynamic param that contains a dash returns as 404 for example [my-token].get.ts
On Nuxt side (routing) it is normalized to mytoken

Couldn't find documented anywhere not to use kebab case.

Would maybe suggest normalizing it like Nuxt does (with frontend routing) - just to ensure consistency between the two.

@danielroe any thoughts?

Thanks so much for your work!

Additional context

No response

Logs

No response

@moshetanzer moshetanzer changed the title can't use kebab case as server route param can't use kebab-case as server route param Nov 11, 2024
@pi0 pi0 changed the title can't use kebab-case as server route param Warn if scaned route params are not supported Nov 11, 2024
@pi0 pi0 added enhancement New feature or request dx and removed pending triage labels Nov 11, 2024
@pi0
Copy link
Member

pi0 commented Nov 11, 2024

I think for nitro api rotues, we should explicitly warn if params are not supported (inside scanner). radix3 and rou3 for Nitro v3 have intentionally more restrictions to front-end routers like vue-router.

@moshetanzer
Copy link
Contributor Author

will add to docs a bit later and open pr - if i understand correctly kebab-case not supported. what should api call return?

@pi0 pi0 added bug Something isn't working and removed enhancement New feature or request dx labels Nov 11, 2024
@pi0
Copy link
Member

pi0 commented Nov 11, 2024

Update: Seems a bug with scanner. runtime matcher supports - as param name.

You can use a custom config like this to allow param test-param:

export default defineNitroConfig({
  handlers: [
    {
      route: "/test/:test-param",
      handler: "routes/test/[test-param].ts",
    },
  ],
});

@moshetanzer
Copy link
Contributor Author

closed other warning pr then - is this the issue then .replace(/\[(\w+)]/g, ":$1") should it be replaced with this .replace(/\[([\w-]+)]/g, ":$1") to allow the dash?

@moshetanzer
Copy link
Contributor Author

would be happy to test - sorry to bug by asking. How can i get nitro working locally - for some reason when running pnpm install I get failed better-sqlite error?

@pi0
Copy link
Member

pi0 commented Nov 11, 2024

I made #2872 as fix in the meantime, and should be shortly fixed in nightly channel.


I think you need to update your Node.js version (to 20 works for me) then use pnpm build --stub and then you can use pnpm nitro dev playground or pnpm vitest test/presets/node.test.ts to run tests against at least one preset. (if you had any other issues re setting up dev, please feel free to reach out 👍🏼 )

@pi0 pi0 changed the title Warn if scaned route params are not supported route files with - in param name are not scanned Nov 11, 2024
@pi0 pi0 closed this as completed in #2872 Nov 11, 2024
@pi0
Copy link
Member

pi0 commented Nov 11, 2024

Thanks for report and reproduction!

@moshetanzer
Copy link
Contributor Author

thanks you are amazing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants