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

Remove necessity for next to always be defined when using .run #215

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sf185127
Copy link

It is possible to hit a scenario where there is no "next" function but if I want to have universal middleware it gets an invalid function back from next-connect when i try to call it.

IE:

const withGlobal = () =>
  createRouter().use(async (req, res, next) => {
    const nextResults = next ? await next() : undefined;
    return {
      ...nextResults,
      props: { ...nextResults?.props, global: { yo: "yo" } },
    };
  });

and in the page i have

export const getServerSideProps = async (ctx: GetServerSidePropsContext) =>
  await createRouter()
    .use(withGlobal())
    .run(ctx.req, ctx.res);

Running this throws TypeError: Function.prototype.apply was called on undefined, which is a undefined and not a function

which i traced back to this file in Router.exec line 80 it expects there to be a function inside fns at ++i but since i'm using run (and not a get or post handler) i shouldnt need to add anymore fns on the end of this.

FYI

export const getServerSideProps = async (ctx: GetServerSidePropsContext) =>
  await createRouter()
    .use(withGlobal())
    .use(() => ({}))
    .run(ctx.req, ctx.res);

does work because creates the fn that next() is looking for, but its ugly and idontwanna

It is possible to hit a scenario where there is no "next" function but if I want to have universal middleware it gets an invalid function back from next-connect when i try to call it.

IE:
```
const withGlobal = () =>
  createRouter().use(async (req, res, next) => {
    const nextResults = next ? await next() : undefined;
    return {
      ...nextResults,
      props: { ...nextResults?.props, global: { yo: "yo" } },
    };
  });
```
and in the page i have
```
export const getServerSideProps = async (ctx: GetServerSidePropsContext) =>
  await createRouter()
    .use(withGlobal())
    .run(ctx.req, ctx.res);
```

Running this throws `TypeError: Function.prototype.apply was called on undefined, which is a undefined and not a function`

which i traced back to this file in Router.exec line 80
it expects there to be a function inside fns at ++i but since i'm using run (and not a get or post handler) i shouldnt need to add anymore fns on the end of this.

FYI 
```
export const getServerSideProps = async (ctx: GetServerSidePropsContext) =>
  await createRouter()
    .use(withGlobal())
    .use(() => ({}))
    .run(ctx.req, ctx.res);
```
*does* work because creates the fn that next() is looking for, but its ugly and idontwanna
@changeset-bot
Copy link

changeset-bot bot commented Nov 13, 2022

⚠️ No Changeset found

Latest commit: e8344ed

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@codecov
Copy link

codecov bot commented Nov 13, 2022

Codecov Report

Merging #215 (e8344ed) into main (e5ac7fa) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #215   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines          436       436           
  Branches        60        60           
=========================================
  Hits           436       436           
Impacted Files Coverage Δ
src/router.ts 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants