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

Types of getStaticPaths & getStaticProps needlessly enforce promises #24684

Closed
ljosberinn opened this issue May 1, 2021 · 1 comment · Fixed by #24685
Closed

Types of getStaticPaths & getStaticProps needlessly enforce promises #24684

ljosberinn opened this issue May 1, 2021 · 1 comment · Fixed by #24685
Milestone

Comments

@ljosberinn
Copy link
Contributor

What version of Next.js are you using?

10.2.0

What version of Node.js are you using?

16

What browser are you using?

Chrome

What operating system are you using?

Windows

How are you deploying your application?

unrelated

Describe the Bug

All code related to typing getStaticPaths and getStaticProps currently enforce having async functions or using the individual types over the convenient GetStaticProps/GetStaticPaths types.

https://github.com/vercel/next.js/blob/0af3b52640/packages/next/types/index.d.ts#L110
https://github.com/vercel/next.js/blob/0af3b52640/packages/next/types/index.d.ts#L116
https://github.com/vercel/next.js/blob/0af3b52640/packages/next/types/index.d.ts#L132

However, it's a perfectly valid use case to use getStaticProps and/or getStaticPaths without loading something from external sources, which wouldn't require an async function.

This is of course really just a nitpick.

Expected Behavior

import type { GetStaticPaths, GetStaticProps } from 'next';
type Report = null | { id: string };

export const getStaticPaths: GetStaticPaths = () => {
  return {
    fallback: true,
    paths: [],
  };
};

export const getStaticProps: GetStaticProps<{ report: Report }> = (ctx) => {
  if (!ctx.params?.id || !isValidReportId(ctx.params.id)) {
    return {
      props: {
        report: null,
      },
    };
  }

  return {
    props: {
      report: {
        report: ctx.params.id,
      },
    },
  };
};

Neither should error, since these work:

export const getStaticPaths = (): GetStaticPathsResult => {
  return {
    fallback: true,
    paths: [],
  };
};

export const getStaticProps = (
  ctx: GetStaticPropsContext
): GetStaticPropsResult<{ report: Report }> => {
  if (!ctx.params?.id || !isValidReportId(ctx.params.id)) {
    return {
      props: {
        report: null,
      },
    };
  }

  return {
    props: {
      report: {
        report: ctx.params.id,
      },
    },
  };
};

To Reproduce

image
image

@ljosberinn ljosberinn added the bug Issue was opened via the bug report template. label May 1, 2021
@timneutkens timneutkens added kind: bug and removed bug Issue was opened via the bug report template. labels May 3, 2021
@timneutkens timneutkens added this to the backlog milestone May 3, 2021
@kodiakhq kodiakhq bot closed this as completed in #24685 May 25, 2021
kodiakhq bot pushed a commit that referenced this issue May 25, 2021
## Bug

- [x] fixes #24684
- [x] Integration tests added

Intentionally omitted changing the types for `GetServerSideProps` etc. as its imo less reasonable to leverage SSR with a sync `getServerSideProps`. Can of course change the type too if you consider that also a valid case.
flybayer pushed a commit to blitz-js/next.js that referenced this issue Jun 16, 2021
…l#24685)

## Bug

- [x] fixes vercel#24684
- [x] Integration tests added

Intentionally omitted changing the types for `GetServerSideProps` etc. as its imo less reasonable to leverage SSR with a sync `getServerSideProps`. Can of course change the type too if you consider that also a valid case.
@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants