hoangvvo / next-connect

The TypeScript-ready, minimal router and middleware layer for Next.js, Micro, Vercel, or Node.js http/http2
https://www.npmjs.com/package/next-connect
MIT License
1.63k stars 65 forks source link

Pattern match not working #131

Open forza11879 opened 3 years ago

forza11879 commented 3 years ago

I am trying to integrate next-connect into my project but for some reason I am not getting consistent results while calling the endpoint. Here is the structure of the endpoints I am calling pages->api->category . Within category folder I have following files index.js - http://localhost:3000/api/category all.js - http://localhost:3000/api/category/all [slug].js - http://localhost:3000/api/category/slug

While I am fetching GET- http://localhost:3000/api/category/all for some reason I am getting GET- http://localhost:3000/api/category/slug which is the wrong one. Nine times out of ten I am hitting the right endpoint.

Here is the apiRoute

import dbMiddleware from './db';
import nextConnect from 'next-connect';

const apiRoute = nextConnect({
  onError(error, req, res) {
    res
      .status(501)
      .json({ error: `Sorry something Happened! ${error.message}` });
  },
  onNoMatch(req, res) {
    res.status(405).json({ error: `Method '${req.method}' Not Allowed` });
  },
}).use(dbMiddleware);

export default apiRoute;
hoangvvo commented 3 years ago

If that is the case, it seems to be a Next.js issue since it is not deterministic in deciding whether to run all.js or [slug].js. Try not using next-connect to see if the issue persists

forza11879 commented 3 years ago

I tried to not use next-connect and it works fine.

On Sun, Apr 11, 2021, 7:58 AM Hoang @.***> wrote:

If that is the case, it seems to be a Next.js issue since it is not deterministic in deciding whether to run all.js or [slug].js. Try not using next-connect to see if the issue persists

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/hoangvvo/next-connect/issues/131#issuecomment-817295543, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFP67SPKZY6FHPO7M63QVFLTIGFHBANCNFSM42T4LN4Q .

hoangvvo commented 3 years ago

Can you please provide a minimal reproduction? Thanks!

forza11879 commented 3 years ago

I will send it to you next week. Just want to fix few bugs.

andersnylund commented 3 years ago

Think I stumbled upon the same problem, where I tried to defined a sub-route along other methods in one handler:

/pages/api/profile.ts:

const handler = nc<NextApiRequest, NextApiResponse>({ onError, onNoMatch })
  .use(authorizedRoute)
  .get(getProfile)
  .put(updateProfile)
  .put("/specific-route", specificHandler)
  .delete(deleteProfile)

This didn't work and resulted that calling /api/profile/specific-route returned 404 as NextJS didn't seem to find it.

To fix it I kept the /pages/api/profile.ts file as is, and created a new file /pages/api/profile/specific-route.ts where I added the route I wanted

mustafaKamal-fe commented 3 years ago

Hope this helps: nextjs catch-all and optional catch-all essentially do this:

catch-all

if i have a route like /api/users/[...slug].js this means

  1. it will catch /api.users/a

  2. it will catch /api/users/a/b

    optinal catch-all

    if i have a route like /api/users/[[...slug]].js

  3. it will catch /api/users/

  4. it will catch /api/users/a

  5. it will catch /api/users/a/b

So far my best bet would be to follow Nextjs routing by using a seperate file for each possible route and start control from there. The reason is: When i used a catch-all approach and placed all my routing logic inside it like this :

// /api/users/[...slug].js

1- const handler = nc();
2- 
3- handler.use('/api/users/, rootMiniapp);
4- handler.use('/api/users/:id', idMiniApp);
5- handlers.use('/api/users/somethingelse', otherMiniapp);
6-
7- export default handler

this caused all possible urls to fall under this route as if they were all accepted . Hence, next-connect was looking through urls one after another (lines 3 to 5 above), once it finds one that fully match will follow it, otherwise it would take unwanted ones and try to match them to one the clossest (usually /api/users/:id) and this is truly bad !!.

Also i tried to apply a midlleware to descide if the requested url actually is among the ones we use and otherwise error out with 404 but this was not ideal and caused to to try to end request from two different places and issue an error.

Please if you find this useless or have a better solution, i am looking forward to know more about it.

Note: See this repo Here

jordant97 commented 1 year ago

Stumble across this issue where i have 2 distinct route but the result will randomly return from either one of the route. (Fetching from postman) Kinda positive that the pattern matching is somewhat buggy.

The route:

  1. pages/api/players/[...slug].ts
  2. pages/api/games/[...slug].ts
ds1371dani commented 1 year ago

I think I also have this problem, structure of my project is as following:

The weird problem is that when I deploy it, at first there is no problem, but after sometime my /api/product requests are sent to [param].ts file instead of the product.ts.

danielmeeusen commented 1 year ago

@ds1371dani

You need to put "[param].ts" into it's own folder so Next.js won't confuse it for any other route.

ds1371dani commented 1 year ago

@ds1371dani

You need to put "[param].ts" into it's own folder so Next.js won't confuse it for any other route.

I need it to be in the same folder to handle routes other than healthcheck and product. This isn't a problem with Next.js, as you can see here there is an order for dynamic routes