Skip to content

Commit

Permalink
fix(nextjs): Remove experimental wrapping of getStaticPaths (#5561)
Browse files Browse the repository at this point in the history
This removes `getStaticPaths` from our WIP data-fetchers auto wrapping experiment. According to the nextjs docs[1], unlike `getStaticProps`, it never runs at runtime, so it shouldn't have been included to begin with.

[1] https://nextjs.org/docs/basic-features/data-fetching/get-static-paths#when-does-getstaticpaths-run
  • Loading branch information
lobsterkatie authored Aug 12, 2022
1 parent a6eee54 commit fbb44d5
Show file tree
Hide file tree
Showing 9 changed files with 19 additions and 62 deletions.
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

0 comments on commit fbb44d5

Please sign in to comment.