-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
ref(nextjs): Remove all deprecated API #10549
Conversation
size-limit report 📦
|
startTransactionOnPageLoad: boolean = true, | ||
startTransactionOnLocationChange: boolean = true, | ||
shouldInstrumentPageload: boolean, | ||
shouldInstrumentNavigation: boolean, | ||
startPageloadSpanCallback: StartSpanCb, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l: We can refactor this later, but IMHO we can simplify this by just passing the client in here and directly calling the start span methods from core here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't follow ^^ Do you mind elaborating? Or do you have examples where we do something similar?
packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts
Outdated
Show resolved
Hide resolved
packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts
Outdated
Show resolved
Hide resolved
@@ -37,8 +36,8 @@ export function wrapRouteHandlerWithSentry<F extends (...args: any[]) => any>( | |||
}); | |||
return continueTrace( | |||
{ | |||
sentryTrace: sentryTraceHeader ?? headers?.get('sentry-trace') ?? undefined, | |||
baggage: baggageHeader ?? headers?.get('baggage'), | |||
sentryTrace: headers?.get('sentry-trace') ?? undefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this not the same as leaving the ?? undefined
away here? (or is it bad if this returns null
, not sure if this is possible) - just because we don't do it below :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit ridiculous but continueTrace
doesn't take null
as sentryTrace
value so we need to turn it into undefined. I wanted to change that anyhow. I'll clean this up in a follow-up.
@mydea I am blocked to migrate |
Removes all deprecated code from the Next.js package.