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.64k stars 65 forks source link

Using next-connect in getServerSideProps removes functionality from the request object #154

Closed bramvbilsen closed 3 years ago

bramvbilsen commented 3 years ago

I am trying to use some middleware in getServerSideProps with next-connect as follows:

export const getServerSideProps: GetServerSideProps = async ({
  req: apiReq,
  res: apiRes,
  locale,
}) => {
  // next-connect usage
  await common().run(apiReq, apiRes);
  if (apiReq.user) {
    return {
      redirect: {
        destination: "/",
        permanent: false,
      },
    };
  } else {
    return {
      props: {
        translations: getTranslations("login", locale),
      },
    };
  }
};

the common() returns a chain of nc().use(...).use(...).... If I then try to redirect, I get the following error:
res.redirect is not a function
If I try to make a redirect without using next-connect (the common method) in getServerSideProps, it does work.
Is this intended behavior? And if so, how can I circumvent this?

hoangvvo commented 3 years ago

Is there anything in common() that calls the method res.redirect? res.redirect is not available inside GetServerSideProps. I meant something like:

const common = () => nc().use((req, res) => { res.redirect(...) }).use(...)...
bramvbilsen commented 3 years ago

Is there anything in common() that calls the method res.redirect? res.redirect is not available inside GetServerSideProps. I meant something like:

const common = () => nc().use((req, res) => { res.redirect(...) }).use(...)...

Sorry for the late response! Yes, I use some middleware that do use redirects so I guess that's what is going wrong. But this middleware is not written by me and out of my control. Is there any reason why redirects are not available? Because it seems like I could add this functionality to the res object myself, but I guess that if the NextJS team didn't add it, there must be some reason why it's not in there. Any idea?

hoangvvo commented 3 years ago

Yeah it is Next.js decision not to have it actually. It is also not possible to call any response function inside GetServerSideProps anyway (like res.end), since it breaks GetServerSideProps (because it wants to process what we return from the function) so res.redirect would not work either way.

hoangvvo commented 3 years ago

@bramvbilsen If that middleware is not of your control, maybe we can work around it by creating a "mock" function that looks like res.redirect([status,] path) (https://nextjs.org/docs/api-routes/response-helpers) like below:

const common = nc.use((req, res) => {
  // note: this middleware should be put first
  res.redirect = (status, path) => {
    if (typeof status === "string") path = status;
    req._redirect = path;
  }
})

Now whenever that your middleware calls redirect, we would attach a property to req. Now back into the getServerSideProps

export const getServerSideProps: GetServerSideProps = async ({
  req: apiReq,
  res: apiRes,
  locale,
}) => {
  // next-connect usage
  await common().run(apiReq, apiRes);
  // If a middleware has called res.redirect, the apiReq._redirect below is available
  if (apiReq._redirect) {
    return {
      redirect: {
        desitination: apiReq._redirect
      }
    }
  }
  // ....
};
bramvbilsen commented 3 years ago

@bramvbilsen If that middleware is not of your control, maybe we can work around it by creating a "mock" function that looks like res.redirect([status,] path) (https://nextjs.org/docs/api-routes/response-helpers) like below:

const common = nc.use((req, res) => {
  // note: this middleware should be put first
  res.redirect = (status, path) => {
    if (typeof status === "string") path = status;
    req._redirect = path;
  }
})

Now whenever that your middleware calls redirect, we would attach a property to req. Now back into the getServerSideProps

export const getServerSideProps: GetServerSideProps = async ({
  req: apiReq,
  res: apiRes,
  locale,
}) => {
  // next-connect usage
  await common().run(apiReq, apiRes);
  // If a middleware has called res.redirect, the apiReq._redirect below is available
  if (apiReq._redirect) {
    return {
      redirect: {
        desitination: apiReq._redirect
      }
    }
  }
  // ....
};

Such a clever way to get around the restriction! My one concern is that most middleware methods return the redirect, something like:

(req, res, next) => {
    return res.redirect("...")
}

Wouldn't that cancel out all following middleware?

hoangvvo commented 3 years ago

Yes, because if next() is not called, the followed middleware will not be run.

bramvbilsen commented 3 years ago

Yes, because if next() is not called, the followed middleware will not be run.

Exactly, so I don't think this will solve it because there's multiple middleware methods that might redirect.

hoangvvo commented 3 years ago

I am not sure I understand the issue here. Once a res.redirect() is called, we would not want the followed middlewares to run right?

bramvbilsen commented 3 years ago

I am not sure I understand the issue here. Once a res.redirect() is called, we would not want the followed middlewares to run right?

You're correct! I got a little confused there. Thanks for the help :)

hoangvvo commented 3 years ago

I am not sure I understand the issue here. Once a res.redirect() is called, we would not want the followed middlewares to run right?

You're correct! I got a little confused there. Thanks for the help :)

Awesome, I guess the issue can be closed. Please let me know/reopen the issue if I can help any furthur!