diff --git a/.changeset/fix-adapter-route-span-nesting.md b/.changeset/fix-adapter-route-span-nesting.md new file mode 100644 index 0000000..3f6eda8 --- /dev/null +++ b/.changeset/fix-adapter-route-span-nesting.md @@ -0,0 +1,17 @@ +--- +"@tindalabs/blindspot-vue": patch +"@tindalabs/blindspot-svelte": patch +"@tindalabs/blindspot-next": patch +--- + +Fix: in-route activity (clicks, fetch, errors, form submits) now nests under the +navigation span instead of orphaning into its own root trace. + +The Vue, Svelte, and Next.js (pages-router) integrations cleared the route span +in a step separate from re-creating it — Vue/Svelte in a `beforeEach` / +`beforeNavigate` guard, the Next pages-router on `routeChangeStart` — leaving a +window where the active context was root. Any span started in that window (and +intermittently the navigation-triggering click) became a standalone root trace. +The clear and re-create now happen atomically when the new route span is +established (`afterEach` / `afterNavigate` / `routeChangeComplete`), so a route +span is active at all times — matching the React and Next app-router adapters. diff --git a/packages/next/src/pages-router.tsx b/packages/next/src/pages-router.tsx index 751f9a6..d4e017b 100644 --- a/packages/next/src/pages-router.tsx +++ b/packages/next/src/pages-router.tsx @@ -29,12 +29,13 @@ export function BlindspotPagesRouter() { setRouteSpan(span); prevPath.current = initialPath; - function onRouteChangeStart() { - clearRouteSpan(); - } - function onRouteChangeComplete(url: string) { const from = prevPath.current; + // End the previous route span and open the new one atomically here, on + // routeChangeComplete. Clearing on routeChangeStart instead left the route + // context root for the whole navigation, so in-route activity that fired + // during the transition orphaned into its own root trace. + clearRouteSpan(); const nextSpan = getTracer().startSpan( `navigation ${from} → ${url}`, { @@ -49,11 +50,9 @@ export function BlindspotPagesRouter() { prevPath.current = url; } - router.events.on('routeChangeStart', onRouteChangeStart); router.events.on('routeChangeComplete', onRouteChangeComplete); return () => { - router.events.off('routeChangeStart', onRouteChangeStart); router.events.off('routeChangeComplete', onRouteChangeComplete); clearRouteSpan(); }; diff --git a/packages/next/test/pages-router.test.tsx b/packages/next/test/pages-router.test.tsx index 5b83fbb..7868c2f 100644 --- a/packages/next/test/pages-router.test.tsx +++ b/packages/next/test/pages-router.test.tsx @@ -73,22 +73,29 @@ describe('BlindspotPagesRouter', () => { ); }); - it('registers routeChangeStart and routeChangeComplete handlers', () => { + it('registers only a routeChangeComplete handler (no separate routeChangeStart clear)', () => { render(); const events = mockRouterEvents.on.mock.calls.map(([e]) => e); - expect(events).toContain('routeChangeStart'); expect(events).toContain('routeChangeComplete'); + expect(events).not.toContain('routeChangeStart'); }); - it('clears the span on routeChangeStart', () => { + it('clears then re-sets the route span atomically within routeChangeComplete', () => { render(); - const handler = getEventHandler('routeChangeStart'); + const handler = getEventHandler('routeChangeComplete'); vi.clearAllMocks(); + mockStartSpan.mockReturnValue({ end: vi.fn(), setAttribute: vi.fn(), addEvent: vi.fn() }); - act(() => { handler?.(); }); + act(() => { handler?.('/about'); }); + // Clearing the previous span here (not on routeChangeStart) keeps a route + // span active through the navigation, so in-route activity nests. expect(mockClearRouteSpan).toHaveBeenCalledTimes(1); + expect(mockSetRouteSpan).toHaveBeenCalledTimes(1); + expect(mockClearRouteSpan.mock.invocationCallOrder[0]!).toBeLessThan( + mockSetRouteSpan.mock.invocationCallOrder[0]!, + ); }); it('creates a new span on routeChangeComplete', () => { @@ -118,8 +125,8 @@ describe('BlindspotPagesRouter', () => { unmount(); - expect(mockRouterEvents.off).toHaveBeenCalledWith('routeChangeStart', expect.any(Function)); expect(mockRouterEvents.off).toHaveBeenCalledWith('routeChangeComplete', expect.any(Function)); + expect(mockRouterEvents.off).not.toHaveBeenCalledWith('routeChangeStart', expect.any(Function)); expect(mockClearRouteSpan).toHaveBeenCalledTimes(1); }); }); diff --git a/packages/svelte/src/router.ts b/packages/svelte/src/router.ts index e9b29bd..f12370d 100644 --- a/packages/svelte/src/router.ts +++ b/packages/svelte/src/router.ts @@ -26,15 +26,18 @@ export function installBlindspotRouter(hooks: NavigationHooks): void { let isFirst = true; let prevPath = ''; - hooks.beforeNavigate(() => { - clearRouteSpan(); - }); - hooks.afterNavigate((nav) => { const search = nav.to.url.search; const toPath = nav.to.url.pathname + (search ? search : ''); const trigger = isFirst ? 'initial' : 'user'; isFirst = false; + + // End the previous route span and open the new one atomically, so a route + // span is active at all times. Clearing in a separate beforeNavigate left a + // window where getRouteContext() was root — in-route clicks/fetch/errors + // fired in that gap orphaned into their own root trace instead of nesting. + clearRouteSpan(); + const parentContext = trigger === 'initial' ? loadRouteContextAfterReload() : undefined; const span = getTracer().startSpan( diff --git a/packages/svelte/test/router.test.ts b/packages/svelte/test/router.test.ts index a5ef0ef..64a878f 100644 --- a/packages/svelte/test/router.test.ts +++ b/packages/svelte/test/router.test.ts @@ -23,9 +23,12 @@ import type { NavigationEvent } from '../src/router.js'; function makeHooks() { let beforeHook: ((nav: { cancel: () => void }) => void) | undefined; let afterHook: ((nav: NavigationEvent) => void) | undefined; + let beforeRegistered = false; return { + get beforeRegistered() { return beforeRegistered; }, beforeNavigate(fn: (nav: { cancel: () => void }) => void) { + beforeRegistered = true; beforeHook = fn; }, afterNavigate(fn: (nav: NavigationEvent) => void) { @@ -75,15 +78,28 @@ describe('installBlindspotRouter', () => { expect(mockSetRouteSpan).toHaveBeenCalledTimes(1); }); - it('clears the previous span on beforeNavigate', () => { + it('does not register a beforeNavigate guard (avoids the root-context gap)', () => { + const hooks = makeHooks(); + installBlindspotRouter(hooks); + // Clearing in a separate beforeNavigate ended the route span before + // afterNavigate re-set it — in-route activity in that gap orphaned. + expect(hooks.beforeRegistered).toBe(false); + }); + + it('clears then re-sets the route span atomically within afterNavigate', () => { const hooks = makeHooks(); installBlindspotRouter(hooks); hooks.navigate(null, '/home'); - vi.clearAllMocks(); - mockLoadRouteContextAfterReload.mockReturnValue(undefined); - hooks.navigate('/home', '/about'); expect(mockClearRouteSpan).toHaveBeenCalledTimes(1); + expect(mockSetRouteSpan).toHaveBeenCalledTimes(1); + expect(mockClearRouteSpan.mock.invocationCallOrder[0]!).toBeLessThan( + mockSetRouteSpan.mock.invocationCallOrder[0]!, + ); + + hooks.navigate('/home', '/about'); + expect(mockClearRouteSpan).toHaveBeenCalledTimes(2); + expect(mockSetRouteSpan).toHaveBeenCalledTimes(2); }); it('records from/to path on subsequent navigation', () => { diff --git a/packages/vue/src/router.ts b/packages/vue/src/router.ts index 0a8a531..d3058c5 100644 --- a/packages/vue/src/router.ts +++ b/packages/vue/src/router.ts @@ -10,14 +10,18 @@ export function installBlindspotRouter(router: Router): void { let isFirst = true; let prevPath = ''; - router.beforeEach(() => { - clearRouteSpan(); - }); - router.afterEach((to) => { const toPath = to.fullPath; const trigger = isFirst ? 'initial' : 'user'; isFirst = false; + + // End the previous route span and open the new one atomically, so a route + // span is active at all times. Doing the clear in a separate beforeEach left + // a window where getRouteContext() was root — any click/fetch/error fired in + // that gap orphaned into its own root trace instead of nesting under the + // route. Mirrors the React adapter's single-effect clear → create → set. + clearRouteSpan(); + const parentContext = trigger === 'initial' ? loadRouteContextAfterReload() : undefined; const span = getTracer().startSpan( diff --git a/packages/vue/test/router.test.ts b/packages/vue/test/router.test.ts index 6c76ee1..f255274 100644 --- a/packages/vue/test/router.test.ts +++ b/packages/vue/test/router.test.ts @@ -22,13 +22,15 @@ import { installBlindspotRouter } from '../src/router.js'; function makeRouter() { let beforeHook: (() => void) | undefined; let afterHook: ((to: { fullPath: string }) => void) | undefined; + let beforeRegistered = false; return { - beforeEach(fn: () => void) { beforeHook = fn; }, + beforeEach(fn: () => void) { beforeRegistered = true; beforeHook = fn; }, afterEach(fn: (to: { fullPath: string }) => void) { afterHook = fn; }, navigate(to: string) { beforeHook?.(); afterHook?.({ fullPath: to }); }, + get beforeRegistered() { return beforeRegistered; }, }; } @@ -57,15 +59,31 @@ describe('installBlindspotRouter', () => { expect(mockSetRouteSpan).toHaveBeenCalledTimes(1); }); - it('clears the previous span on beforeEach', () => { + it('does not register a beforeEach guard (avoids the root-context gap)', () => { + const router = makeRouter(); + installBlindspotRouter(router as never); + // A separate beforeEach clear ended the route span before afterEach re-set + // it, leaving a window where getRouteContext() was root — in-route clicks/ + // fetch/errors fired in that gap orphaned into their own root traces. + expect(router.beforeRegistered).toBe(false); + }); + + it('clears then re-sets the route span atomically within afterEach', () => { const router = makeRouter(); installBlindspotRouter(router as never); router.navigate('/home'); - vi.clearAllMocks(); - mockLoadRouteContextAfterReload.mockReturnValue(undefined); - router.navigate('/about'); + // Both happen in the single afterEach callback, clear before set, so a route + // span is active at all times and in-route activity nests under it. expect(mockClearRouteSpan).toHaveBeenCalledTimes(1); + expect(mockSetRouteSpan).toHaveBeenCalledTimes(1); + expect(mockClearRouteSpan.mock.invocationCallOrder[0]!).toBeLessThan( + mockSetRouteSpan.mock.invocationCallOrder[0]!, + ); + + router.navigate('/about'); + expect(mockClearRouteSpan).toHaveBeenCalledTimes(2); + expect(mockSetRouteSpan).toHaveBeenCalledTimes(2); }); it('records the from/to path on subsequent navigation', () => {