Skip to content

Commit

Permalink
fix(nextjs): Make NextApiHandler type version-agnostic (#5737)
Browse files Browse the repository at this point in the history
In `withSentry` in the nextjs SDK, we import and use the `NextApiHandler` type provided by Next. In vercel/next.js#35166, which was released in 12.1.6, that type switched to using `uknown` rather than `void` for the function's return type.

For users this isn't a problem. If they import both the type from Nextjs and `withSentry` from our SDK (which in turn imports the type), both imports will be pointed at their local version of Next, and the types will match. For us, though, it's a different story. Our dev version of Next is the lowest one the SDK supports, 10.0.8. If we link our SDK into a test app, the `NextApiHandler` we're importing will be coming from the `node_modules` in our repo, whereas the test app will be pulling the type from its own `node_modules`. And that works just fine... until you try to upgrade your test app to Next 12.1.6 or higher, at which point your app starts refusing to build because of the mismatch.

Though at first it might seem that the obvious solution is just to update our dev version of Next to 12.1.6+, if we do that we'll just be back in the same place with the same problem trying to use lower versions of Next in our test apps. Thus we need the type to work for both older and newer versions, which is what this PR does.
  • Loading branch information
lobsterkatie committed Sep 14, 2022
1 parent fbd4ef0 commit 180f644
Showing 1 changed file with 20 additions and 5 deletions.
25 changes: 20 additions & 5 deletions packages/nextjs/src/utils/withSentry.ts
Expand Up @@ -10,11 +10,26 @@ import {
stripUrlQueryAndFragment,
} from '@sentry/utils';
import * as domain from 'domain';
import { NextApiHandler, NextApiRequest, NextApiResponse } from 'next';

// This is the same as the `NextApiHandler` type, except instead of having a return type of `void | Promise<void>`, it's
// only `Promise<void>`, because wrapped handlers are always async
export type WrappedNextApiHandler = (req: NextApiRequest, res: NextApiResponse) => Promise<void>;
import { NextApiRequest, NextApiResponse } from 'next';

// These are the same as the official `NextApiHandler` type, except
//
// a) The wrapped version returns only promises, because wrapped handlers are always async.
//
// b) Instead of having a return types based on `void` (Next < 12.1.6) or `unknown` (Next 12.1.6+), both the wrapped and
// unwrapped versions of the type have both. This doesn't matter to users, because they exist solely on one side of that
// version divide or the other. For us, though, it's entirely possible to have one version of Next installed in our
// local repo (as a dev dependency) and have another Next version installed in a test app which also has the local SDK
// linked in.
//
// In that case, if those two versions are on either side of the 12.1.6 divide, importing the official `NextApiHandler`
// type here would break the test app's build, because it would set up a situation in which the linked SDK's
// `withSentry` would refer to one version of the type (from the local repo's `node_modules`) while any typed handler in
// the test app would refer to the other version of the type (from the test app's `node_modules`). By using a custom
// version of the type compatible with both the old and new official versions, we can use any Next version we want in
// a test app without worrying about type errors.
type NextApiHandler = (req: NextApiRequest, res: NextApiResponse) => void | Promise<void> | unknown | Promise<unknown>;
export type WrappedNextApiHandler = (req: NextApiRequest, res: NextApiResponse) => Promise<void> | Promise<unknown>;

export type AugmentedNextApiResponse = NextApiResponse & {
__sentryTransaction?: Transaction;
Expand Down

0 comments on commit 180f644

Please sign in to comment.