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

withPageAuthRequired for getServerSideProps breaks typing for InferGetServerSidePropsType #309

Closed
GeeWee opened this issue Feb 20, 2021 · 5 comments · Fixed by #512
Closed
Labels
help wanted Extra attention is needed

Comments

@GeeWee
Copy link

GeeWee commented Feb 20, 2021

Describe the problem

Using next.js and Typescript there is a utility method (described here) that allows you to automatically infer the types of props based on the return type of getServerSideProps.

Example of how it works normally:

export const getServerSideProps = async (context: GetServerSidePropsContext) => {
    return {
      props: {
        myProp: true,
      },
    };
  }

export const MyComponent: React.FC<InferGetServerSidePropsType<typeof getServerSideProps>> = (props) =>{
  // Allowed!
  console.log(props.myProp)
  
  // Not allowed!
  console.log(props.wrongProp)
  return <></>
}

However this does not work when getServerSideProps is wrapped in `withPageAuthRequired - the props are no longer typed properly.

Reproduction case

export const getServerSideProps = withPageAuthRequired({
  getServerSideProps: async (context: GetServerSidePropsContext) => {
    return {
      props: {
        myProp: true,
      },
    };
  },
});

export const MyComponent: React.FC<InferGetServerSidePropsType<typeof getServerSideProps>> = (
  props,
) => {
  // Yep and type-checked
  console.log(props.user);

  // Yep, but type "any"
  console.log(props.myProp);

  // Yep (but wrong! (and type any))
  console.log(props.wrongProp);
  return <></>;
};

What was the expected behavior?

I expect that the "original" prop return type is union'ed together with the user prop, so that the access is type-safe for both.

Environment

  • Version of this library used: 1.0.0
  • Which framework are you using, if applicable: next@^10.0.7
@adamjmcgrath
Copy link
Contributor

Hi @GeeWee - thanks for raising this, it would definitely be nice to support InferGetServerSidePropsType here.

I expect that the "original" prop return type is union'ed together with the user prop, so that the access is type-safe for both.

Do you have an idea on how this would work? I can't find any good example code

const wrap = (fn) => fn; // <= how to type `wrap` so that props.myProp is inferred

export const getServerSideProps = wrap(async () => {
  return {
    props: {
      myProp: true
    }
  };
});

export const MyComponent: React.FC<InferGetServerSidePropsType<typeof getServerSideProps>> = (props) => {
  console.log(props.myProp); // Should be inferred
  return <></>;
};

@adamjmcgrath adamjmcgrath added the help wanted Extra attention is needed label Feb 22, 2021
@GeeWee
Copy link
Author

GeeWee commented Feb 22, 2021

I actually tried fiddling around with it for a bit as well, but I wasn't able to get some typings that I liked.
A workaround solution could probably be to define your own InferGetServerSideProspWithUser which is essentially just the type from the "regular" GetServerSideProps but with a user Union'd into it. However that's obviously not optimal, so I'm hoping someone else has a better idea that'll allow InferGetServerSidePropsType to "just" work.

@DopamineDriven
Copy link

Try explicitly setting GetServerSidePropsResult -- I have to do that for a 100% accurate inference when using it with InferGetStaticProps. Here is an example implementation (much easier than trying to build your own wrapper function imo)

getStaticProps (note that the props are implicitly returned via addApolloState(apolloClient, { ... }) wrapper, this works in instances of explicitly returning them as well)

export async function getStaticProps(
	ctx: GetStaticPropsContext
): Promise<
	GetStaticPropsResult<{
		TopLevelPage: ParentPageQuery['TopLevelPage'];
		Header: DynamicNavQuery['Header'];
		Footer: DynamicNavQuery['Footer'];
	}>
> {
	const params = ctx.params!;
	const apolloClient = initializeApollo();
	await apolloClient.query<
		ParentPageQuery,
		ParentPageQueryVariables
	>({
		query: ParentPageDocument,
		variables: {
			idType: WordpressPageIdType.URI,
			id: encodeURIComponent(params.paths![0] as string),
			size: WordpressMediaItemSizeEnum.LARGE
		}
	});
	await apolloClient.query<
		DynamicNavQuery,
		DynamicNavQueryVariables
	>({
		query: DynamicNavDocument,
		variables: {
			idHead: 'Header',
			idTypeHead: WordpressMenuNodeIdTypeEnum.NAME,
			idTypeFoot: WordpressMenuNodeIdTypeEnum.NAME,
			idFoot: 'Footer'
		}
	});

	return addApolloState(apolloClient, {
		props: {},
		revalidate: 60
	});
}

Default function inferring props explicitly

function WordPressCatchAll({
	TopLevelPage,
	Header,
	Footer
}: InferGetStaticPropsType<typeof getStaticProps>) {
	const router = useRouter();
	const paths = router.query;
	return (
		<>
			{router.isFallback ? (
				<Fallback />
			) : (
				<AppLayout
					Header={Header}
					Footer={Footer}
					title={`${paths.paths![0]}` ?? 'The Fade Room Inc.'}
				>
					<Container clean className='fit'>
						<PageTemplate topLevelPage={TopLevelPage} />
					</Container>
				</AppLayout>
			)}
		</>
	);
}

export default WordPressCatchAll;

you can do this with getServerSideProps, I have done it in other repos, with explicit type inference confirmed by intellisense when hovering over the props

definition from installed next package in ./node_modules/next/types/index.d.ts

export type GetServerSidePropsContext<
  Q extends ParsedUrlQuery = ParsedUrlQuery
> = {
  req: IncomingMessage & {
    cookies: NextApiRequestCookies
  }
  res: ServerResponse
  params?: Q
  query: ParsedUrlQuery
  preview?: boolean
  previewData?: any
  resolvedUrl: string
  locale?: string
  locales?: string[]
  defaultLocale?: string
}

export type GetServerSidePropsResult<P> =
  | { props: P }
  | { redirect: Redirect }
  | { notFound: true }

export type GetServerSideProps<
  P extends { [key: string]: any } = { [key: string]: any },
  Q extends ParsedUrlQuery = ParsedUrlQuery
> = (
  context: GetServerSidePropsContext<Q>
) => Promise<GetServerSidePropsResult<P>>

export type InferGetServerSidePropsType<T> = T extends GetServerSideProps<
  infer P,
  any
>
  ? P
  : T extends (
      context?: GetServerSidePropsContext<any>
    ) => Promise<GetServerSidePropsResult<infer P>>
  ? P
  : never

This def indicates that it is ultimately looking for Promise<GetServerSidePropsResult<infer P>> ? P : never
you also remove a layer of conditional checking by opting into the explcit Promise Result method.

I have even built out custom typedefs recently to use this with getInitialProps since next is yet to provide those in their package as utility types...and it works, as confirmed by intellisense, allows for an explicit typesafe injection of header/footer on the very first render. it's their server-to-client under the hood consumption pattern, most beautiful part of nextjs in my opinion.

_app.tsx

NextApp.getInitialProps = async (
	appContext: AppContext
): Promise<
	GetInitialPropsResult<{
		pageProps: AppInitialProps;
		Header: DynamicNavQuery['Header'];
		Footer: DynamicNavQuery['Footer'];
	}>
> => {
	const pageProps = await App.getInitialProps(appContext);
	const graphqlClient = initializeApollo();
	const dynamicNav = await graphqlClient.query<
		DynamicNavQuery,
		DynamicNavQueryVariables
	>({
		query: DynamicNavDocument,
		variables: {
			idHead: 'Header',
			idTypeHead: WordpressMenuNodeIdTypeEnum.NAME,
			idTypeFoot: WordpressMenuNodeIdTypeEnum.NAME,
			idFoot: 'Footer'
		}
	});
	const Header = dynamicNav.data.Header;
	const Footer = dynamicNav.data.Footer;
	return addApolloState(graphqlClient, {
		pageProps: {
			...pageProps.pageProps,
			Header,
			Footer
		}
	});
};

function NextApp({
	Component,
	pageProps: { ...pageProps }
}: AppProps<
	typeof NextApp.getInitialProps
>): React.ReactElement<AppProps> {
// do stuff...
}

export default NextApp

@mikestopcontinues
Copy link

When I'm not using Auth0, this is how I match the types:

type Props = {a: string};
type Query = {a: string};

export default function PageRoute(props: Props) {
  return (<div />);
}

export const getServerSideProps: GetServerSideProps<Props, Query> = 
  async ({req: {query: {a}}}) => ({props: {a}});

So the easiest option I see is to allow statically typing withPageAuthRequired, and provide a Props wrapper like all React (PropsWithChildren PropsWithRef):

type Props = {a: string};
type Query = {a: string};

export default function PageRoute(props: PropsWithPageAuthRequired<Props>) {
  return (<div />);
}

export const getServerSideProps = withPageAuthRequired<Props, Query>({
  async getServerSideProps({req: {query: {a}}}) => ({props: {a}}),
});

@DopamineDriven
Copy link

DopamineDriven commented May 27, 2021

That makes sense @mikestopcontinues, however the more detailed way of breaking down GetServerSideProps into its constituent GetServerSidePropsContext and GetServerSidePropsResult utility types has consistently yielded 100% type inference on my end (whereas not explicitly listing return types has caused typescript to throw errors).

I do, however, have an idea as to how to approach this issue for people getting stuck with the auth wrapper. Using Module Augmentation to wrap getServerSideProps GetServerSideProps with the type of withPageAuthRequired -- make a next.d.ts file in your types directory and declare module 'next' { /* ultimately this */ typeof withPageAuthRequired extends GetServerSideProps> } -- similar to how you're able to incorporate specific properties/types into pageProps without having to use getInitialProps at all for next/app:

types/next.d.ts

import type { NextComponentType, NextPageContext } from 'next';
import type { Session } from 'next-auth';
import type { Router } from 'next/router';
import { DynamicNavQuery } from '@/graphql/generated/graphql';
declare module 'next/app' {
	type AppProps<P = Record<string, unknown>> = {
		Component: NextComponentType<NextPageContext, any, P>;
		router: Router;
		__N_SSG?: boolean;
		__N_SSP?: boolean;
		pageProps: P & {
			/** Initial session passed in from `getServerSideProps` or `getInitialProps` */
			session?: Session;
		};
	};
}

The only suggestion I'd make for your default function is as follows (to get 100% type inference via intellisense in development)

export default function PageRoute<T extends typeof getServerSideProps>({ a  }: InferGetServerSidePropsType<T>) {
         return (<div />);
}

The above gives fantastic type inference with 100% type retention from server to client and no reinjecting it with Props

export async function getServerSideProps<P>(
	ctx: GetServerSidePropsContext
): Promise<
	GetServerSidePropsResult<
		P & {
			Header: DynamicNavQuery['Header'];
			Footer: DynamicNavQuery['Footer'];
			user: WordpressUser | null;
			getter: Session | null;
			nookieUser: Session | null;
		}
	>
> {    

//...
   return {
         props: { a }
      }
};

that said, the InferGetServerSideProps utility type tends to only work if you use the GetServerSidePropsResult utility type and explicitly list your return types.

Anyway, I think module augmentation might be the best approach for incorporating any auth wrapper -- have the withPageAuthRequired Type wrap GetServerSideProps type in a .d.ts file then you can have two separate GetServerSideProps utility types -- one with auth one without.

Here's the getServerSideProps type breakdown; embedding GetServerSideProps within the auth wrapper type should work I think 👀 let me know if you end up trying it and it does! curious to find out

export type GetServerSidePropsContext<
  Q extends ParsedUrlQuery = ParsedUrlQuery
> = {
  req: IncomingMessage & {
    cookies: NextApiRequestCookies
  }
  res: ServerResponse
  params?: Q
  query: ParsedUrlQuery
  preview?: boolean
  previewData?: any
  resolvedUrl: string
  locale?: string
  locales?: string[]
  defaultLocale?: string
}

export type GetServerSidePropsResult<P> =
  | { props: P }
  | { redirect: Redirect }
  | { notFound: true }

export type GetServerSideProps<
  P extends { [key: string]: any } = { [key: string]: any },
  Q extends ParsedUrlQuery = ParsedUrlQuery
> = (
  context: GetServerSidePropsContext<Q>
) => Promise<GetServerSidePropsResult<P>>

export type InferGetServerSidePropsType<T> = T extends GetServerSideProps<
  infer P,
  any
>
  ? P
  : T extends (
      context?: GetServerSidePropsContext<any>
    ) => Promise<GetServerSidePropsResult<infer P>>
  ? P
  : never

When I'm not using Auth0, this is how I match the types:

type Props = {a: string};
type Query = {a: string};

export default function PageRoute(props: Props) {
  return (<div />);
}

export const getServerSideProps: GetServerSideProps<Props, Query> = 
  async ({req: {query: {a}}}) => ({props: {a}});

So the easiest option I see is to allow statically typing withPageAuthRequired, and provide a Props wrapper like all React (PropsWithChildren PropsWithRef):

type Props = {a: string};
type Query = {a: string};

export default function PageRoute(props: PropsWithPageAuthRequired<Props>) {
  return (<div />);
}

export const getServerSideProps = withPageAuthRequired<Props, Query>({
  async getServerSideProps({req: {query: {a}}}) => ({props: {a}}),
});

Widcket added a commit that referenced this issue Oct 7, 2021
Widcket added a commit that referenced this issue Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants