From 180f6449ec463e013e702c633f0dcba75815006d Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 14 Sep 2022 11:43:16 -0700 Subject: [PATCH] fix(nextjs): Make `NextApiHandler` type version-agnostic (#5737) In `withSentry` in the nextjs SDK, we import and use the `NextApiHandler` type provided by Next. In https://github.com/vercel/next.js/pull/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. --- packages/nextjs/src/utils/withSentry.ts | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/packages/nextjs/src/utils/withSentry.ts b/packages/nextjs/src/utils/withSentry.ts index d1b249bf87a3..0229d91135cc 100644 --- a/packages/nextjs/src/utils/withSentry.ts +++ b/packages/nextjs/src/utils/withSentry.ts @@ -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`, it's -// only `Promise`, because wrapped handlers are always async -export type WrappedNextApiHandler = (req: NextApiRequest, res: NextApiResponse) => Promise; +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 | unknown | Promise; +export type WrappedNextApiHandler = (req: NextApiRequest, res: NextApiResponse) => Promise | Promise; export type AugmentedNextApiResponse = NextApiResponse & { __sentryTransaction?: Transaction;