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

API middleware running on 2 seperate API endpoints #141

Open waleedshabbir1 opened 3 years ago

waleedshabbir1 commented 3 years ago

I have implemented next-connect

I tried to divide the common next-connect login into files and I just import them whenever I need to use handler

This is my nextConnect.js file `import nc from "next-connect";

export default nc({ onError(error, req, res) { res.status(501).json({ error: Sorry something happened!! }); }, onNoMatch(req, res) { res.status(405).json({ error: Method ${req.method} Not Allowed }); }, }); `

and this is the file where I import and use the handler

`import handler from "../../../utils/nextConnect/handler"; import { fetchData } from "../../../utils/apiControllers/login";

export default handler .post(async (req, res) => { const notes = await fetchData(req, res); }); `

I call API likes this

  1. login
  2. Other API
  3. Login

When I call Login API 2nd time after calling Other API. I get a response from the Other API. I am not sure why this is happening. Also I tried to use different middleware with my API this is the same problem with them.

hoangvvo commented 3 years ago

The method above is incorrect because it is mutating the same next-connect instance. https://github.com/hoangvvo/next-connect/issues/106#issuecomment-688695209

For example, let's say you import the same handler in two places and both adds to it a handler handler.post, the two will accidentally override each other.

So the idea is to always create a new nc instance for each route.

If you have a common set of middleware or options, do something like below instead.

// common.js
export const common = nc().use(one).use(two);
export const options = {
  onError(error, req, res) {
    res.status(501).json({ error: Sorry something happened!! });
  },
  onNoMatch(req, res) {
    res.status(405).json({ error: Method ${req.method} Not Allowed });
  },
}
// pages/api/foo.js
import { common, options } from "../common";
const handler = nc(options).use(common).get().post();
// pages/api/bar.js
import { common, options } from "../common";
const handler = nc(options).use(common).get().post();
waleedshabbir1 commented 3 years ago

So this means importing next-connect in every route I want to use

hoangvvo commented 3 years ago

Yup, you need to create a new next-connect instance every time.

waleedshabbir1 commented 3 years ago

Thank you. It was confusing when migrating from expressJs

waleedshabbir1 commented 3 years ago

I tried the above approach, It works fine for my original problem However, options are not working when I use middleware.

const handler = nc(options).use(newPasswordValidator,validationResults).post(async (req, res) => { const notes = await fetchData(req, res); });

If I try to send GET request it still use middleware and latter post API

hoangvvo commented 3 years ago

It should not be like that. Can you post the full code snippet?

waleedshabbir1 commented 3 years ago

common.js

import nc from "next-connect";

export const options = {
  onError(error, req, res) {
    res.status(501).json({ error: `Sorry something happened!!` });
  },
  onNoMatch(req, res) {
    res.status(405).json({ error: `Method ${req.method} Not Allowed` });
  },
}

API.js

import {options} from "../../../utils/nextConnect/common";
import nc from 'next-connect'
import {validationResults,newPasswordValidator} from "../../../utils/middleware/reqValidator";

const handler = nc(options).use(newPasswordValidator,validationResults).post(async (req, res) =>
 { const notes = await fetchData(req, res); });

  export default handler;

If I remove .use(middleware) part then options are consumed otherwise not

waleedshabbir1 commented 3 years ago

I think the problem is middleware is used before the post request. so next-connect isn't aware if GET method exists or not. So it will always run middleware before no matter what type of request is GET/POST/PUT etc

I was experimenting I tried to do like this const handler = nc(options).post(newPasswordValidator,validationResults,async (req, res) => { const notes = await fetchData(req, res); });

Somehow, this works but I am not sure if its the right way to do. This is how I used to do in expressJS also

hoangvvo commented 3 years ago

I don't think that is the case. Do you make sure to call next() in both newPasswordValidator,validationResults?

waleedshabbir1 commented 3 years ago

yes

hoangvvo commented 3 years ago

Is it possible for you to post a minimum reproduction? @waleedshabbir1

waleedshabbir1 commented 3 years ago

The above code is the example of it.

However If I use it like this it works fine const handler = nc(options).post(newPasswordValidator,validationResults, async (req, res) => { const notes = await fetchData(req, res); });

hoangvvo commented 3 years ago

Could you post the content of newPasswordValidator and validationResults?