search

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

Crash when using a a next-connect handler shared between API routes and pages #106

Closed arnaudriegert closed 3 years ago

arnaudriegert commented 3 years ago

Hi, I've been using next-connect in a few projects and I encountered a problem, for which I found a workaround while writing the minimal non-working example, but I thought I should still mention it to you because I don't really understand the issue.

The problem arises when I fetch an API from the client side and that I have a shared middleware that I want to apply both in pages and in API routes.

Here's a non-working example: https://gist.github.com/arnaudpeich/cfe6e9379a09f10c9044f9121af8d475

I boiled it down to the minimum that makes the bug occur, but obviously in real projects I use real middleware, not just the base handler.

This first load of the index page works fine, but then when I refresh, I get this res.json is not a function error even on the server side (which seems to indicate that on the reload, the handler for /api/nothing is called even though I'm requesting /index and I'm only calling the API route from the client-side).

It works OK if instead of using the same handler, I create a new handler and use the shared one... but I'm not sure why that is.

hoangvvo commented 3 years ago

Thanks for the reproduction.

Both in your API Routes and page use the same nc instance. When you call baseMiddleware.get in API Routes to attach the handler to that instance, the one used in page also has that GET handler because it is the same instance (baseMiddleware).

So when a GET request come to the page, it runs that getter function defined in the API Routes that calls res.json() which does not exist in page.

It is better to go with use for sharing middleware.

arnaudriegert commented 3 years ago

Ok thanks, that's what I ended up doing. I didn't quite get why the getter would be called on a route where the path didn't match, but I can see how sharing the same instance of nc would lead to that. Anyway the issue is closed as far as I'm concerned, thanks again :)

iagowp commented 3 years ago

Hey, I've been having this issue for a long time and had no idea why. (I think a bad next-js course taught this). Maybe this information that the instance should not be shared could be on the readme? I don't know if that's a common enough mistake for it to be worth it, but I can tell I was very frustrated with it (I thought it was some issue with next itself, not this package)

shansana commented 3 years ago

I somehow managed to share instance between api routes by using a module. Refer this answer https://stackoverflow.com/a/67320953