Skip to content

Commit

Permalink
stop wrapping getStaticPaths
Browse files Browse the repository at this point in the history
  • Loading branch information
lobsterkatie committed Aug 10, 2022
1 parent 64c7204 commit 2fa6855
Show file tree
Hide file tree
Showing 9 changed files with 19 additions and 59 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 @@ -207,7 +207,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 @@ -288,8 +288,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)) {
// console.log(node);
// debugger;
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`.
*/

import { logger } from '@sentry/utils';
Expand All @@ -23,7 +22,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 @@ -91,7 +89,7 @@ function wrapFunctions(userCode: string, templateCode: string, filepath: string)
}

/**
* Wrap `getStaticPaths`, `getStaticProps`, and `getServerSideProps` (if they exist) in the given page code
* Wrap `getStaticProps` and `getServerSideProps` (if they exist) in the given page code
*/
export default function wrapDataFetchersLoader(this: LoaderThis<LoaderOptions>, userCode: string): string {
// We know one or the other will be defined, depending on the version of webpack being used
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 @@ -28,7 +23,3 @@ export const getStaticProps =
typeof __ORIG_GSPROPS__ === 'function'
? ServerSideSentryNextjsSDK.withSentryGSProps(__ORIG_GSPROPS__)
: __ORIG_GSPROPS__;
export const getStaticPaths =
typeof __ORIG_GSPATHS__ === 'function'
? ServerSideSentryNextjsSDK.withSentryGSPaths(__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 `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,3 +1,2 @@
export { withSentryGSPaths } from './withSentryGSPaths';
export { withSentryGSProps } from './withSentryGSProps';
export { withSentryGSSP } from './withSentryGSSP';
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,24 +2,13 @@ import type {
GetServerSideProps,
GetServerSidePropsContext,
GetServerSidePropsResult,
GetStaticPaths,
GetStaticPathsContext,
GetStaticPathsResult,
GetStaticProps,
GetStaticPropsContext,
GetStaticPropsResult,
} 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 @@ -34,4 +23,4 @@ export type GSSP = {
result: GetServerSidePropsResult<Props>;
};

export type DataFetchingFunction = GSPaths | GSProps | GSSP;
export type DataFetchingFunction = GSProps | GSSP;
16 changes: 0 additions & 16 deletions packages/nextjs/src/config/wrappers/withSentryGSPaths.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 `getStaticProps` and `getServerSideProps`
* @param origFunction The user's exported `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;
}
2 changes: 1 addition & 1 deletion packages/nextjs/src/index.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ function addServerIntegrations(options: NextjsOptions): void {
export type { SentryWebpackPluginOptions } from './config/types';
export { withSentryConfig } from './config';
export { isBuild } from './utils/isBuild';
export { withSentryGSProps, withSentryGSSP, withSentryGSPaths } from './config/wrappers';
export { withSentryGSProps, withSentryGSSP } 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 2fa6855

Please sign in to comment.