Skip to content

fix(nextjs): Remove experimental wrapping of getStaticPaths #5561

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

Merged
merged 1 commit into from
Aug 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/nextjs/src/config/loaders/ast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ function maybeRenameNode(ast: AST, identifierPath: ASTPath<IdentifierNode>, alia

// In general we want to rename all nodes, unless we're in one of a few specific situations. (Anything which doesn't
// get handled by one of these checks will be renamed at the end of this function.) In all of the scenarios below,
// we'll use `gSSP` as our stand-in for any of `getServerSideProps`, `getStaticProps`, and `getStaticPaths`.
// we'll use `gSSP` as our stand-in for either of `getServerSideProps` and `getStaticProps`.

// Imports:
//
Expand Down Expand Up @@ -296,8 +296,8 @@ function maybeRenameNode(ast: AST, identifierPath: ASTPath<IdentifierNode>, alia
//
// Second, because need to wrap the object using its local name, we need to rename `local`. This tracks with how we
// thought about `import` statements above, but is different from everything else we're doing in this function in that
// it means we potentially need to rename something *not* already named `getServerSideProps`, `getStaticProps`, or
// `getStaticPaths`, meaning we need to rename nodes outside of the collection upon which we're currently acting.
// it means we potentially need to rename something *not* already named `getServerSideProps` or `getStaticProps`,
// meaning we need to rename nodes outside of the collection upon which we're currently acting.
if (ExportSpecifier.check(parent)) {
if (parent.exported.name !== parent.local?.name && node === parent.exported) {
const currentLocalName = parent.local?.name || '';
Expand Down
16 changes: 7 additions & 9 deletions packages/nextjs/src/config/loaders/dataFetchersLoader.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
/**
* This loader auto-wraps a user's page-level data-fetching functions (`getStaticPaths`, `getStaticProps`, and
* `getServerSideProps`) in order to instrument them for tracing. At a high level, this is done by finding the relevant
* functions, renaming them so as not to create a name collision, and then creating a new version of each function which
* is a wrapped version of the original. We do this by parsing the user's code and some template code into ASTs,
* manipulating them, and then turning them back into strings and appending our template code to the user's (modified)
* page code. Greater detail and explanations can be found in situ in the functions below and in the helper functions in
* `ast.ts`.
* This loader auto-wraps a user's page-level data-fetching functions (`getStaticProps` and `getServerSideProps`) in
* order to instrument them for tracing. At a high level, this is done by finding the relevant functions, renaming them
* so as not to create a name collision, and then creating a new version of each function which is a wrapped version of
* the original. We do this by parsing the user's code and some template code into ASTs, manipulating them, and then
* turning them back into strings and appending our template code to the user's (modified) page code. Greater detail and
* explanations can be found in situ in the functions below and in the helper functions in `ast.ts`.
*
* For `getInitialProps` we create a virtual proxy-module that re-exports all the exports and default exports of the
* original file and wraps `getInitialProps`. We do this since it allows us to very generically wrap `getInitialProps`
Expand Down Expand Up @@ -34,7 +33,6 @@ import type { LoaderThis } from './types';
const DATA_FETCHING_FUNCTIONS = {
getServerSideProps: { placeholder: '__ORIG_GSSP__', alias: '' },
getStaticProps: { placeholder: '__ORIG_GSPROPS__', alias: '' },
getStaticPaths: { placeholder: '__ORIG_GSPATHS__', alias: '' },
};

type LoaderOptions = {
Expand Down Expand Up @@ -101,7 +99,7 @@ function wrapFunctions(userCode: string, templateCode: string, filepath: string)
}

/**
* Wrap `getStaticPaths`, `getStaticProps`, and `getServerSideProps` (if they exist) in the given page code
* Wrap `getInitialProps`, `getStaticProps`, and `getServerSideProps` (if they exist) in the given page code
*/
export default function wrapDataFetchersLoader(this: LoaderThis<LoaderOptions>, userCode: string): string {
// For now this loader only works for ESM code
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
import type {
GetServerSideProps as GetServerSidePropsFunction,
GetStaticPaths as GetStaticPathsFunction,
GetStaticProps as GetStaticPropsFunction,
} from 'next';
import type { GetServerSideProps as GetServerSidePropsFunction, GetStaticProps as GetStaticPropsFunction } from 'next';

declare const __ORIG_GSSP__: GetServerSidePropsFunction;
declare const __ORIG_GSPROPS__: GetStaticPropsFunction;
declare const __ORIG_GSPATHS__: GetStaticPathsFunction;

// We import the SDK under a purposefully clunky name, to lessen to near zero the chances of a name collision in case
// the user has also imported Sentry for some reason. (In the future, we could check for such a collision using the AST,
Expand All @@ -30,7 +25,3 @@ export const getStaticProps =
typeof __ORIG_GSPROPS__ === 'function'
? ServerSideSentryNextjsSDK.withSentryGetStaticProps(__ORIG_GSPROPS__)
: __ORIG_GSPROPS__;
export const getStaticPaths =
typeof __ORIG_GSPATHS__ === 'function'
? ServerSideSentryNextjsSDK.withSentryGetStaticPaths(__ORIG_GSPATHS__)
: __ORIG_GSPATHS__;
3 changes: 1 addition & 2 deletions packages/nextjs/src/config/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ export type UserSentryOptions = {
widenClientFileUpload?: boolean;

experiments?: {
// Automatically wrap `getServerSideProps`, `getStaticProps`, and `getStaticPaths` in order to instrument them for
// tracing.
// Automatically wrap `getInitialProps`, `getServerSideProps`, and `getStaticProps` in order to instrument them for tracing.
autoWrapDataFetchers?: boolean;
};
};
Expand Down
1 change: 0 additions & 1 deletion packages/nextjs/src/config/wrappers/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
export { withSentryGetStaticPaths } from './withSentryGetStaticPaths';
export { withSentryGetStaticProps } from './withSentryGetStaticProps';
export { withSentryGetInitialProps } from './withSentryGetInitialProps';
export { withSentryGetServerSideProps } from './withSentryGetServerSideProps';
13 changes: 1 addition & 12 deletions packages/nextjs/src/config/wrappers/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,15 @@ import type {
GetServerSideProps,
GetServerSidePropsContext,
GetServerSidePropsResult,
GetStaticPaths,
GetStaticPathsContext,
GetStaticPathsResult,
GetStaticProps,
GetStaticPropsContext,
GetStaticPropsResult,
NextPage,
NextPageContext,
} from 'next';

type Paths = { [key: string]: string | string[] };
type Props = { [key: string]: unknown };

export type GSPaths = {
fn: GetStaticPaths;
wrappedFn: GetStaticPaths;
context: GetStaticPathsContext;
result: GetStaticPathsResult<Paths>;
};

export type GSProps = {
fn: GetStaticProps;
wrappedFn: GetStaticProps;
Expand All @@ -43,4 +32,4 @@ export type GIProps = {
result: unknown;
};

export type DataFetchingFunction = GSPaths | GSProps | GSSP | GIProps;
export type DataFetchingFunction = GSProps | GSSP | GIProps;
14 changes: 0 additions & 14 deletions packages/nextjs/src/config/wrappers/withSentryGetStaticPaths.ts

This file was deleted.

10 changes: 5 additions & 5 deletions packages/nextjs/src/config/wrappers/wrapperUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,22 @@ import { DataFetchingFunction } from './types';
* Pass-through wrapper for the original function, used as a first step in eventually wrapping the data-fetching
* functions with code for tracing.
*
* @template T Types for `getStaticPaths`, `getStaticProps`, and `getServerSideProps`
* @param origFunction The user's exported `getStaticPaths`, `getStaticProps`, or `getServerSideProps` function
* @template T Types for `getInitialProps`, `getStaticProps`, and `getServerSideProps`
* @param origFunction The user's exported `getInitialProps`, `getStaticProps`, or `getServerSideProps` function
* @param context The context object passed by nextjs to the function
* @returns The result of calling the user's function
*/
export async function callOriginal<T extends DataFetchingFunction>(
origFunction: T['fn'],
context: T['context'],
): Promise<T['result']> {
let pathsOrProps;
let props;

// TODO: Can't figure out how to tell TS that the types are correlated - that a `GSPropsFunction` will only get passed
// `GSPropsContext` and never, say, `GSSPContext`. That's what wrapping everything in objects and using the generic
// and pulling the types from the generic rather than specifying them directly was supposed to do, but... no luck.
// eslint-disable-next-line prefer-const, @typescript-eslint/no-explicit-any
pathsOrProps = await (origFunction as any)(context);
props = await (origFunction as any)(context);

return pathsOrProps;
return props;
}
7 changes: 1 addition & 6 deletions packages/nextjs/src/index.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,7 @@ function addServerIntegrations(options: NextjsOptions): void {
export type { SentryWebpackPluginOptions } from './config/types';
export { withSentryConfig } from './config';
export { isBuild } from './utils/isBuild';
export {
withSentryGetServerSideProps,
withSentryGetStaticProps,
withSentryGetStaticPaths,
withSentryGetInitialProps,
} from './config/wrappers';
export { withSentryGetServerSideProps, withSentryGetStaticProps, withSentryGetInitialProps } from './config/wrappers';
export { withSentry } from './utils/withSentry';

// Wrap various server methods to enable error monitoring and tracing. (Note: This only happens for non-Vercel
Expand Down