Skip to content

feat(react): Add TanStack Router integration #12095

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.

Sign up for GitHub

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 10 commits into from
May 29, 2024
Merged

feat(react): Add TanStack Router integration #12095

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7cd66ce
Initial TanStack Router Integration
MicahLyle May 17, 2024
2983c42
Clean things up a bit
May 28, 2024
7c839ed
Add E2E tests
May 28, 2024
af12b07
fix
May 28, 2024
a3a2008
Merge branch 'develop' into develop
May 28, 2024
fbf0247
Add playwright dep
May 28, 2024
a27f2da
There goes an hour I'll never get back
May 28, 2024
17242cc
Use new onBeforeNavigate hook
May 29, 2024
2e9ca18
Merge branch 'develop' into develop
May 29, 2024
8a5b592
Implement review feedback
May 29, 2024
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
71 changes: 71 additions & 0 deletions packages/react/src/tanstackrouter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { WINDOW, startBrowserTracingNavigationSpan, startBrowserTracingPageLoadSpan } from "@sentry/browser";
import {
SEMANTIC_ATTRIBUTE_SENTRY_OP,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
} from "@sentry/core";

import { browserTracingIntegration as originalBrowserTracingIntegration } from "@sentry/react";
import type { Client, Integration } from "@sentry/types";

import type { TanstackRouter } from "./types";

/**
* A custom browser tracing integration for Tanstack Router.
*/
export function tanstackRouterBrowserTracingIntegration(
router: TanstackRouter,
options: Parameters<typeof originalBrowserTracingIntegration>[0] = {}
): Integration {
const browserTracingIntegrationInstance = originalBrowserTracingIntegration({
...options,
instrumentNavigation: false,
instrumentPageLoad: false,
});

const { instrumentPageLoad = true, instrumentNavigation = true } = options;

return {
...browserTracingIntegrationInstance,
afterAllSetup(client) {
const initPathName = WINDOW && WINDOW.location && WINDOW.location.pathname;
if (instrumentPageLoad && initPathName) {
startBrowserTracingPageLoadSpan(client, {
name: initPathName,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there no way for us to get the routeId for the pageload? We can read this from router.state also right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I believe so. Sorry, I missed putting it there. @lforst can you put that in?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Route ID is on each match. You'll probably want the deepest one: matches[matches.length - 1].routeId
  • Entire route config is available at: router.routesById[routeId]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did the matches[matches.length - 1].routeId approach below, just forgot to put it here as well.

attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: "url",
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: "pageload",
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: "auto.pageload.react.tanstack_router",
},
});
}

if (instrumentNavigation) {
tanstackRouterInstrumentNavigation(router, client);
}

browserTracingIntegrationInstance.afterAllSetup(client);
},
};
}

export function tanstackRouterInstrumentNavigation(router: TanstackRouter, client: Client): void {
router.history.subscribe(() => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure about the scope, but do we not need to unsubscribe at one point?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we can add one, we just need to hook it onto closing the Sentry client, which we currently don't have an exposed hook for in the public API.

const state = router.state;
const matches = state.pendingMatches ?? state.matches;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems like what we do internally is more like this:

const matches = [
  ...state.cachedMatches,
  ...(state.pendingMatches ?? []),
  ...state.matches,
]

see: https://github.com/TanStack/router/blob/bce8e71652acdfa66594fedaab4bc5f058ccaf6e/packages/react-router/src/RouterProvider.tsx#L113-L117

I did find multiple implementations of this with different spreading order, so I'm not sure which one would be correct 😅

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll want one of these depending on the situation:

  • state.pendingMatches || state.matches - The user may be in the process of navigating to a new location, which is why you would want pendingMatches to represent that pending navigation
  • state.matches - If you're only interested in where the user is currently and are not interested in any potential pending navigation destinations, then this should suffice.

I recommend the former. Do not use state.cachedMatches for anything outside of reporting back stuff that's cached.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tannerlinsley I did const matches = state.pendingMatches ?? state.matches;. Would that be worse than state.pendingMatches || state.matches?

const lastMatch = matches[matches.length - 1];
if (!lastMatch) return;

const routeId = lastMatch?.routeId;
if (!routeId) return;

startBrowserTracingNavigationSpan(client, {
name: routeId,
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: "navigation",
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: "auto.navigation.tanstack_router.router_instrumentation",
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: "route",
},
});
});
}
28 changes: 28 additions & 0 deletions packages/react/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,3 +211,31 @@ export type CreateRouterFunction<
TState extends RouterState = RouterState,
TRouter extends Router<TState> = Router<TState>,
> = (routes: RouteObject[], opts?: any) => TRouter;

// Types for @tanstack/react-router >= 1.32.13

export interface TanstackRouter {
history: TanstackRouterHistory;
state: TanstackRouterState;
}

export interface TanstackRouterHistory {
subscribe: (cb: () => void) => () => void;
}

export interface TanstackRouterState {
matches: Array<TanstackRouterRouteMatch>;
pendingMatches?: Array<TanstackRouterRouteMatch>;
}

export interface TanstackRouterRouteMatch {
routeId: string;
pathname: string;
params: { [key: string]: string };
status: "pending" | "success" | "error" | "redirected" | "notFound";
isFetching: false | "beforeLoad" | "loader";
error: unknown;
cause: "preload" | "enter" | "stay";
preload: boolean;
invalid: boolean;
}