Skip to content

fix(nextjs): Start navigation transactions on same-route navigations #5642

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

Merged
merged 4 commits into from
Aug 29, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions packages/nextjs/src/performance/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,20 @@ const DEFAULT_TAGS = {
} as const;

let activeTransaction: Transaction | undefined = undefined;
let prevTransactionName: string | undefined = undefined;
let startTransaction: StartTransactionCb | undefined = undefined;

// We keep track of the previous page location so we can avoid creating transactions when navigating to the same page.
// This variable should always contain a pathname. (without query string or fragment)
// We are making a tradeoff by not starting transactions when just the query string changes. One could argue that we
// should in fact start transactions when the query changes, however, in some cases (for example when typing in a search
// box) the query might change multiple times a second, resulting in way too many transactions.
// Because we currently don't have a real way of preventing transactions to be created in this case (except for the
// shotgun approach `startTransactionOnLocationChange: false`), we won't start transactions when *just* the query changes.
let previousLocation: string | undefined = undefined;

// We keep track of the previous transaction name so we can set the `from` field on navigation transactions.
let prevTransactionName: string | undefined = undefined;

const client = getCurrentHub().getClient();

/**
Expand All @@ -137,6 +148,8 @@ export function nextRouterInstrumentation(
const { route, traceParentData, baggage, params } = extractNextDataTagInformation();

prevTransactionName = route || global.location.pathname;
previousLocation = global.location.pathname;

const source = route ? 'route' : 'url';

activeTransaction = startTransactionCb({
Expand Down Expand Up @@ -197,19 +210,25 @@ function changeStateWrapper(originalChangeStateWrapper: RouterChangeState): Wrap
...args: any[]
): Promise<boolean> {
const newTransactionName = stripUrlQueryAndFragment(url);

// do not start a transaction if it's from the same page
if (startTransaction !== undefined && prevTransactionName !== newTransactionName) {
if (startTransaction !== undefined && previousLocation !== as) {
previousLocation = as;

if (activeTransaction) {
activeTransaction.finish();
}

const tags: Record<string, Primitive> = {
...DEFAULT_TAGS,
method,
...options,
};

if (prevTransactionName) {
tags.from = prevTransactionName;
}

prevTransactionName = newTransactionName;
activeTransaction = startTransaction({
name: prevTransactionName,
Expand Down