From d6e9279398cfe72783d0b8dd7b85bedabf6e1bab Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Fri, 21 Aug 2026 11:25:57 -0400 Subject: [PATCH 1/9] fix(datadog): stop destroying the initial_load view so LCP/FCP report again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under `trackViewsManually` the RUM SDK stays stopped until the first `startView`, adopts that call's options as its one `initial_load` view, and turns every later call into a `route_change` view. Only an `initial_load` view runs `trackInitialViewMetrics`, so it is the only view that can ever carry LCP or FCP. Studio called `startView` twice on boot — once in `useDatadog`, then again in `useOnRouteLoadTracker` — so the initial view was ended microseconds later and its paint metrics were thrown away. Measured against the real SDK bundle, the two calls land 0.3ms apart and the initial_load event ships with dom_complete but no lcp and no fcp, matching production exactly: of 200 initial_load views, dom_complete 17, fcp 2, lcp 0. Drop the `useDatadog` call and leave the single one to `useOnRouteLoadTracker`, which mounts on the root route and so runs on every cloud route. That also fixes the view name: `useDatadog` used `window.location.pathname`, which is permanently `/` under the hash router, so all 880 initial_load views in a week were named `/` regardless of the route actually loaded. Refs #1570 Co-Authored-By: Claude Opus 5 (1M context) --- src/integrations/datadog/datadog.test.tsx | 92 +++++++++++++++++++++++ src/integrations/datadog/datadog.ts | 14 ++-- 2 files changed, 99 insertions(+), 7 deletions(-) create mode 100644 src/integrations/datadog/datadog.test.tsx diff --git a/src/integrations/datadog/datadog.test.tsx b/src/integrations/datadog/datadog.test.tsx new file mode 100644 index 000000000..e202173f0 --- /dev/null +++ b/src/integrations/datadog/datadog.test.tsx @@ -0,0 +1,92 @@ +/** + * @vitest-environment jsdom + */ +import { cleanup, render } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const { rum, routerState } = vi.hoisted(() => ({ + rum: { + init: vi.fn(), + startView: vi.fn(), + onReady: vi.fn((callback: () => void) => callback()), + setUser: vi.fn(), + clearUser: vi.fn(), + addAction: vi.fn(), + }, + routerState: { href: '/#/org-1/cluster-1/apps', params: [{ organizationId: 'org-1' }] }, +})); + +vi.mock('@datadog/browser-rum', () => ({ datadogRum: rum })); +vi.mock('@datadog/browser-rum-react', () => ({ reactPlugin: () => ({ name: 'react' }) })); +vi.mock('@/config/constants', () => ({ isLocalStudio: false })); +vi.mock('@/hooks/useAuth', () => ({ useOverallAuth: () => ({ user: undefined }) })); +vi.mock('@tanstack/react-router', () => ({ + useLocation: () => ({ href: routerState.href }), + useRouter: () => ({ + state: { location: { href: routerState.href } }, + matchRoutes: () => routerState.params.map((params) => ({ params })), + }), +})); + +async function loadDatadogModule() { + vi.resetModules(); + // `enabled` is a module-scope const, so DEV has to be false before the import. + vi.stubEnv('DEV', false); + return import('./datadog'); +} + +afterEach(() => { + cleanup(); + vi.unstubAllEnvs(); + vi.clearAllMocks(); +}); + +describe('useDatadog', () => { + it('initializes RUM', async () => { + const { useDatadog } = await loadDatadogModule(); + function Harness() { + useDatadog(); + return null; + } + + render(); + + expect(rum.init).toHaveBeenCalledTimes(1); + }); + + // The guard for #1570. Under `trackViewsManually` only the FIRST startView becomes an + // `initial_load` view, and only an `initial_load` view collects LCP/FCP — so a startView + // here is destroyed by `useOnRouteLoadTracker`'s call in the same effect flush, taking + // Studio's Core Web Vitals with it. Ownership of that first call must stay in one place. + it("does not start a view — that is useOnRouteLoadTracker's job", async () => { + const { useDatadog } = await loadDatadogModule(); + function Harness() { + useDatadog(); + return null; + } + + render(); + + expect(rum.startView).not.toHaveBeenCalled(); + }); +}); + +describe('useOnRouteLoadTracker', () => { + beforeEach(() => { + routerState.href = '/#/org-1/cluster-1/apps'; + routerState.params = [{ organizationId: 'org-1' }]; + }); + + it('starts exactly one view per render pass, named by route', async () => { + const { useOnRouteLoadTracker } = await loadDatadogModule(); + function Harness() { + useOnRouteLoadTracker(); + return null; + } + + render(); + + expect(rum.startView).toHaveBeenCalledTimes(1); + expect(rum.startView.mock.calls[0][0]).toMatchObject({ name: expect.any(String) }); + }); +}); diff --git a/src/integrations/datadog/datadog.ts b/src/integrations/datadog/datadog.ts index 8dd2e87df..2bbeb0144 100644 --- a/src/integrations/datadog/datadog.ts +++ b/src/integrations/datadog/datadog.ts @@ -42,13 +42,13 @@ export function useDatadog() { plugins: [reactPlugin()], }); - datadogRum.onReady(() => { - datadogRum.startView({ - service: 'studio', - version: import.meta.env.VITE_STUDIO_VERSION, - name: window.location.pathname || 'initial', - }); - }); + // Deliberately no `startView` here. Under `trackViewsManually` the SDK stays stopped + // until the FIRST `startView`, adopts that call's options as its one `initial_load` + // view, and downgrades every later call to `route_change` — and only an `initial_load` + // view ever collects LCP/FCP. A second call therefore destroys the vitals (#1570). That + // one call belongs to `useOnRouteLoadTracker`, which mounts on the root route (so it + // runs on every cloud route) and names the view by route rather than by the hash + // router's permanently-`/` pathname. } }, []); } From 93fbf5001c49f18f1fde5e03328893940d371257 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Fri, 21 Aug 2026 11:33:42 -0400 Subject: [PATCH 2/9] test(datadog): assert the one-boot-view invariant at tree level, and trim comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cross-model review (cursor-composer) noted the #1570 guard rendered `useDatadog` alone, so it only caught the specific regression of re-adding that call — a second `startView` introduced anywhere else in the boot tree would leave it green while production went back to zero vitals. Mount both hooks the way production does (App → StudioCloud) and assert exactly one `startView`, which is the invariant that actually matters; keep the isolated case to narrow a failure to the hook that regressed. Also pin the expected view name to the translated route so a revert to pathname-based naming fails CI instead of silently restoring permanently-`/` names, and trim the comments in both files to the one non-obvious SDK constraint per the repo's zero-new-comments default (codex nit). Both new assertions are mutation-verified: re-adding the deleted `startView` turns the tree-level test and the isolated test red. Refs #1570 Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 30 ++++++++++ src/integrations/datadog/datadog.test.tsx | 67 +++++++++++++---------- src/integrations/datadog/datadog.ts | 9 +-- 3 files changed, 70 insertions(+), 36 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 7da32d029..c255fe77b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -383,6 +383,36 @@ Only fields on the SDK's modifiable-field allowlist can be mutated here. `view.n `error.fingerprint`; resource events add `resource.url` (plus GraphQL/header/websocket fields). Grep the installed bundle for `"view.referrer":"string"` to re-check after an SDK bump. +## Datadog RUM — exactly one `startView` per page load, or Core Web Vitals vanish + +[`datadog.ts`](src/integrations/datadog/datadog.ts) sets `trackViewsManually: true`, and the SDK's +contract for that flag is narrower than it looks. It stays stopped until the **first** `startView`, +adopts that call's options as its single `initial_load` view, then turns every later call into a +`route_change` view (`preStartRum.ts` `tryStartRum`, `trackViews.ts` `startView`). Only an +`initial_load` view runs `trackInitialViewMetrics`, so it is the only view that can ever carry LCP +or FCP. + +A second `startView` on boot therefore ends the one view that collects paint metrics. That is what +zeroed Studio's vitals for a month (#1570): `useDatadog` and `useOnRouteLoadTracker` both called +it, 0.3ms apart, and every `initial_load` event shipped with `dom_complete` but no `lcp` and no +`fcp`. Ownership of that first call now lives in `useOnRouteLoadTracker` alone — it mounts on the +root route (`rootRoute.ts`), so it runs on every cloud route. + +Two traps when reading this from RUM data: + +- `view.time_spent` on an `initial_load` view is measured from `clocksOrigin()` — page origin, not + SDK start — so a ~1s `time_spent` does **not** mean the view was alive and observing for a + second. It says nothing about the observation window. +- Never name a view from `window.location.pathname`. Studio uses hash routing, so that is + permanently `/` — which is why all 880 `initial_load` views in one week were named `/` whatever + route actually loaded, and why #1405's "`/` view regression" was really every deep-link entry + conflated into one bucket. + +Verifying a change here needs a **visible** browser on a production build: a headless or background +tab reports `visibilityState: 'hidden'`, emits zero paint and LCP entries, and `trackFirstHidden` +discards them anyway — so vitals always read as absent, and a broken fix looks identical to a +working one. + ## Never put an address or a credential in a URL Studio uses **hash routing**, so a query param lives in the fragment: for diff --git a/src/integrations/datadog/datadog.test.tsx b/src/integrations/datadog/datadog.test.tsx index e202173f0..c5c4c90a3 100644 --- a/src/integrations/datadog/datadog.test.tsx +++ b/src/integrations/datadog/datadog.test.tsx @@ -13,7 +13,7 @@ const { rum, routerState } = vi.hoisted(() => ({ clearUser: vi.fn(), addAction: vi.fn(), }, - routerState: { href: '/#/org-1/cluster-1/apps', params: [{ organizationId: 'org-1' }] }, + routerState: { href: '/org-1/clu-2/apps', params: [{ organizationId: 'org-1', clusterId: 'clu-2' }] }, })); vi.mock('@datadog/browser-rum', () => ({ datadogRum: rum })); @@ -35,58 +35,67 @@ async function loadDatadogModule() { return import('./datadog'); } +beforeEach(() => { + routerState.href = '/org-1/clu-2/apps'; + routerState.params = [{ organizationId: 'org-1', clusterId: 'clu-2' }]; +}); + afterEach(() => { cleanup(); vi.unstubAllEnvs(); vi.clearAllMocks(); }); -describe('useDatadog', () => { - it('initializes RUM', async () => { - const { useDatadog } = await loadDatadogModule(); - function Harness() { - useDatadog(); +describe('Datadog view tracking', () => { + // Only the first `startView` of a page load becomes an `initial_load` view, and only an + // `initial_load` view collects LCP/FCP — so a second one on boot zeroes Core Web Vitals + // (#1570). This mirrors the production tree: App calls useDatadog, then StudioCloud (the + // root route component) calls useOnRouteLoadTracker. + it('starts exactly one view when the whole tree boots', async () => { + const { useDatadog, useOnRouteLoadTracker } = await loadDatadogModule(); + function CloudRoot() { + useOnRouteLoadTracker(); return null; } + function App() { + useDatadog(); + return ; + } - render(); + render(); expect(rum.init).toHaveBeenCalledTimes(1); + expect(rum.startView).toHaveBeenCalledTimes(1); }); - // The guard for #1570. Under `trackViewsManually` only the FIRST startView becomes an - // `initial_load` view, and only an `initial_load` view collects LCP/FCP — so a startView - // here is destroyed by `useOnRouteLoadTracker`'s call in the same effect flush, taking - // Studio's Core Web Vitals with it. Ownership of that first call must stay in one place. - it("does not start a view — that is useOnRouteLoadTracker's job", async () => { - const { useDatadog } = await loadDatadogModule(); - function Harness() { - useDatadog(); + it("names that view by route, not by the hash router's pathname", async () => { + const { useDatadog, useOnRouteLoadTracker } = await loadDatadogModule(); + function CloudRoot() { + useOnRouteLoadTracker(); return null; } + function App() { + useDatadog(); + return ; + } - render(); + render(); - expect(rum.startView).not.toHaveBeenCalled(); + expect(rum.startView).toHaveBeenCalledWith( + expect.objectContaining({ name: '/$organizationId/$clusterId/apps/' }), + ); }); -}); -describe('useOnRouteLoadTracker', () => { - beforeEach(() => { - routerState.href = '/#/org-1/cluster-1/apps'; - routerState.params = [{ organizationId: 'org-1' }]; - }); - - it('starts exactly one view per render pass, named by route', async () => { - const { useOnRouteLoadTracker } = await loadDatadogModule(); + // Narrows a failure of the tree-level assertion above to the hook that regressed. + it('does not start a view from useDatadog', async () => { + const { useDatadog } = await loadDatadogModule(); function Harness() { - useOnRouteLoadTracker(); + useDatadog(); return null; } render(); - expect(rum.startView).toHaveBeenCalledTimes(1); - expect(rum.startView.mock.calls[0][0]).toMatchObject({ name: expect.any(String) }); + expect(rum.startView).not.toHaveBeenCalled(); }); }); diff --git a/src/integrations/datadog/datadog.ts b/src/integrations/datadog/datadog.ts index 2bbeb0144..ec67e6863 100644 --- a/src/integrations/datadog/datadog.ts +++ b/src/integrations/datadog/datadog.ts @@ -42,13 +42,8 @@ export function useDatadog() { plugins: [reactPlugin()], }); - // Deliberately no `startView` here. Under `trackViewsManually` the SDK stays stopped - // until the FIRST `startView`, adopts that call's options as its one `initial_load` - // view, and downgrades every later call to `route_change` — and only an `initial_load` - // view ever collects LCP/FCP. A second call therefore destroys the vitals (#1570). That - // one call belongs to `useOnRouteLoadTracker`, which mounts on the root route (so it - // runs on every cloud route) and names the view by route rather than by the hash - // router's permanently-`/` pathname. + // No `startView` here: `useOnRouteLoadTracker` must own the sole boot view, because + // only the first one is an `initial_load` view and only that view collects LCP/FCP. } }, []); } From 7b06cbfd43ee52a976b118bc1ffecda08e35da15 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Fri, 21 Aug 2026 11:39:14 -0400 Subject: [PATCH 3/9] test(datadog): cover subsequent-navigation views, and trim test comments Round-2 review (gemini) noted the suite proved the boot case but nothing about later navigations, so a regression that stopped emitting `route_change` views would go unnoticed. Assert the tracker emits a further named view per href change. Comment trim per the repeated nit from both lenses: drop the issue-number narration and the restated test rationale, keeping only the two non-obvious constraints (the module-scope `enabled` read, and the production nesting the boot test mirrors). Refs #1570 Co-Authored-By: Claude Opus 5 (1M context) --- src/integrations/datadog/datadog.test.tsx | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/integrations/datadog/datadog.test.tsx b/src/integrations/datadog/datadog.test.tsx index c5c4c90a3..d81d772cf 100644 --- a/src/integrations/datadog/datadog.test.tsx +++ b/src/integrations/datadog/datadog.test.tsx @@ -47,10 +47,7 @@ afterEach(() => { }); describe('Datadog view tracking', () => { - // Only the first `startView` of a page load becomes an `initial_load` view, and only an - // `initial_load` view collects LCP/FCP — so a second one on boot zeroes Core Web Vitals - // (#1570). This mirrors the production tree: App calls useDatadog, then StudioCloud (the - // root route component) calls useOnRouteLoadTracker. + // The nesting mirrors production: App calls useDatadog, StudioCloud calls the tracker. it('starts exactly one view when the whole tree boots', async () => { const { useDatadog, useOnRouteLoadTracker } = await loadDatadogModule(); function CloudRoot() { @@ -86,7 +83,6 @@ describe('Datadog view tracking', () => { ); }); - // Narrows a failure of the tree-level assertion above to the hook that regressed. it('does not start a view from useDatadog', async () => { const { useDatadog } = await loadDatadogModule(); function Harness() { @@ -98,4 +94,21 @@ describe('Datadog view tracking', () => { expect(rum.startView).not.toHaveBeenCalled(); }); + + it('starts a further view on each subsequent navigation', async () => { + const { useOnRouteLoadTracker } = await loadDatadogModule(); + function CloudRoot() { + useOnRouteLoadTracker(); + return null; + } + + const { rerender } = render(); + routerState.href = '/org-1/clu-2/config'; + rerender(); + + expect(rum.startView.mock.calls.map(([options]) => options.name)).toEqual([ + '/$organizationId/$clusterId/apps/', + '/$organizationId/$clusterId/config/', + ]); + }); }); From 7b065c8f9639369d5ea565a54f6e0220a523080f Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Fri, 21 Aug 2026 11:41:14 -0400 Subject: [PATCH 4/9] style(datadog): drop the last narrating test comment The invariant is documented in AGENTS.md; the nesting is visible in the code. Third repeat of the same review nit. Refs #1570 Co-Authored-By: Claude Opus 5 (1M context) --- src/integrations/datadog/datadog.test.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/integrations/datadog/datadog.test.tsx b/src/integrations/datadog/datadog.test.tsx index d81d772cf..b2ca0828d 100644 --- a/src/integrations/datadog/datadog.test.tsx +++ b/src/integrations/datadog/datadog.test.tsx @@ -47,7 +47,6 @@ afterEach(() => { }); describe('Datadog view tracking', () => { - // The nesting mirrors production: App calls useDatadog, StudioCloud calls the tracker. it('starts exactly one view when the whole tree boots', async () => { const { useDatadog, useOnRouteLoadTracker } = await loadDatadogModule(); function CloudRoot() { From ff3635b04aa4474f2c9a6093d9de9b1175320992 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Fri, 21 Aug 2026 11:47:38 -0400 Subject: [PATCH 5/9] test(datadog): give the mocked router a stable identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gemini-code-assist: the mock returned a fresh `useRouter()` object per render, and the tracker's effect lists `router` in its deps — so the effect re-fired on every render and the navigation assertions held even without `location.href` as a dependency. The real `useRouter` returns a stable reference, so the mock was also unfaithful. Instantiate the router once with a getter for `state`, and assert that a re-render which changes nothing produces no view. That assertion fails against the old unstable mock, so the fix stays guarded. Refs #1570 Co-Authored-By: Claude Opus 5 (1M context) --- src/integrations/datadog/datadog.test.tsx | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/integrations/datadog/datadog.test.tsx b/src/integrations/datadog/datadog.test.tsx index b2ca0828d..12b9ab0eb 100644 --- a/src/integrations/datadog/datadog.test.tsx +++ b/src/integrations/datadog/datadog.test.tsx @@ -20,13 +20,21 @@ vi.mock('@datadog/browser-rum', () => ({ datadogRum: rum })); vi.mock('@datadog/browser-rum-react', () => ({ reactPlugin: () => ({ name: 'react' }) })); vi.mock('@/config/constants', () => ({ isLocalStudio: false })); vi.mock('@/hooks/useAuth', () => ({ useOverallAuth: () => ({ user: undefined }) })); -vi.mock('@tanstack/react-router', () => ({ - useLocation: () => ({ href: routerState.href }), - useRouter: () => ({ - state: { location: { href: routerState.href } }, +// One stable `router` identity, as the real `useRouter` returns — the tracker's effect lists +// `router` in its deps, so a fresh object per render would re-fire it on every render and the +// navigation assertions below would hold even if `location.href` were not a dependency. +vi.mock('@tanstack/react-router', () => { + const router = { + get state() { + return { location: { href: routerState.href } }; + }, matchRoutes: () => routerState.params.map((params) => ({ params })), - }), -})); + }; + return { + useLocation: () => ({ href: routerState.href }), + useRouter: () => router, + }; +}); async function loadDatadogModule() { vi.resetModules(); @@ -102,9 +110,11 @@ describe('Datadog view tracking', () => { } const { rerender } = render(); + rerender(); routerState.href = '/org-1/clu-2/config'; rerender(); + // The middle re-render changed nothing, so it must not have produced a view. expect(rum.startView.mock.calls.map(([options]) => options.name)).toEqual([ '/$organizationId/$clusterId/apps/', '/$organizationId/$clusterId/config/', From b6d33ae8bc4649124151def11a20dd2ba079fe20 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Fri, 21 Aug 2026 11:52:34 -0400 Subject: [PATCH 6/9] docs(agents): note that mocked hooks in dependency arrays need stable identities The lesson from this PR's own escaped review finding: an unstable mocked `useRouter` made an effect-counting test pass for the wrong reason, and would have passed with the dependency removed entirely. Records the getter pattern and the no-op-rerender assertion that catches it. Refs #1570 Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index c255fe77b..c7d3ac8b6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -178,6 +178,34 @@ and collapses. Fix: make the wrapper `flex flex-col` (or add `block`/`w-full` to anchor). The GitHub button (`.github-signin-btn`) sets `display: flex` explicitly and is not affected. (Hit while building the "Last used" sign-in badge, #1316.) +## A mocked hook whose value lands in a dependency array must keep a stable identity + +If you `vi.mock` a hook and the code under test puts its return value in a `useEffect` dependency +array, returning a fresh object per call re-fires that effect on **every render** — and any +assertion counting effect side-effects then passes for the wrong reason. `vi.mock('@tanstack/ +react-router', () => ({ useRouter: () => ({ ... }) }))` did exactly that to +[`datadog.test.tsx`](src/integrations/datadog/datadog.test.tsx): the test that claimed to prove +"one view per navigation" would have passed with `location.href` removed from the deps entirely. +The real `useRouter` returns a stable reference, so the mock was also lying about production. + +Instantiate once in the factory and expose changing state through a getter: + +```ts +vi.mock('@tanstack/react-router', () => { + const router = { + get state() { + return { location: { href: routerState.href } }; + }, + }; + return { useRouter: () => router }; +}); +``` + +The general check, worth running on any effect-counting test: **re-render without changing +anything and assert nothing happened.** That assertion is what distinguishes "the effect re-ran +because its input changed" from "the effect re-runs constantly"; it fails immediately against an +unstable mock. + ## Testing Radix menus in jsdom This repo has NO `@testing-library/user-event` — only `@testing-library/react` + From 8791073a5b7865fabf91ad4fe21855671c5771d3 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Mon, 24 Aug 2026 13:12:48 -0400 Subject: [PATCH 7/9] test(datadog): pin the sole boot view against the real router MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing suite mocks `@tanstack/react-router` entirely, so nothing proved the invariant the fix actually rests on: that the tracker is mounted at the root route and therefore runs on every cloud page load. If someone moves `useOnRouteLoadTracker` into a narrower layout, RUM silently stops starting any view — a no-data failure nothing alerts on. Boots the real `rootRoute` and the real `dashboardLayout` guard on a signed-out deep link and asserts exactly one `startView`, named `/sign-in/`. Mutation-verified both ways: dropping the tracker from `StudioCloud` gives 0 calls, and disabling the redirect guard moves the name to the deep link. It also settles a question the PR description had left to the reviewer — a boot-time redirect does not re-fire the initial view, because the guard resolves during the router's initial load and the root component never commits the deep-link location. Refs #1570 Co-Authored-By: Claude Opus 5 (1M context) --- .../datadog/datadogBootView.test.tsx | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 src/integrations/datadog/datadogBootView.test.tsx diff --git a/src/integrations/datadog/datadogBootView.test.tsx b/src/integrations/datadog/datadogBootView.test.tsx new file mode 100644 index 000000000..0e0185167 --- /dev/null +++ b/src/integrations/datadog/datadogBootView.test.tsx @@ -0,0 +1,74 @@ +/** + * @vitest-environment jsdom + */ +import { QueryClient } from '@tanstack/react-query'; +import { render, waitFor } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +const { rum } = vi.hoisted(() => ({ + rum: { + init: vi.fn(), + startView: vi.fn(), + onReady: vi.fn((callback: () => void) => callback()), + setUser: vi.fn(), + clearUser: vi.fn(), + addAction: vi.fn(), + }, +})); + +vi.mock('@datadog/browser-rum', () => ({ datadogRum: rum })); +vi.mock('@datadog/browser-rum-react', () => ({ reactPlugin: () => ({ name: 'react' }) })); +vi.mock('@/features/notifications/NotificationsSubscriptionManager', () => ({ + NotificationsSubscriptionManager: () => null, +})); +vi.mock('@/components/NotificationBanner', () => ({ NotificationBanner: () => null })); + +// The real root route and the real dashboard guard, with stub leaves: the point is that +// `rootRoute`'s component is what mounts the tracker, so this fails if the tracker ever stops +// being mounted at the root — the failure mode the SDK cannot report, because it is "no data". +async function bootDeepLink() { + vi.resetModules(); + vi.stubEnv('DEV', false); + const router = await import('@tanstack/react-router'); + const { rootRoute } = await import('@/router/rootRoute'); + const { dashboardLayout } = await import('@/router/dashboardRoute'); + const { OverallAppSignIn } = await import('@/features/auth/store/authStore'); + + const deepRoute = router.createRoute({ + getParentRoute: () => dashboardLayout, + path: '/$organizationId/$clusterId/apps', + component: () => null, + }); + const signInRoute = router.createRoute({ + getParentRoute: () => rootRoute, + path: '/sign-in', + component: () => null, + }); + const instance = router.createRouter({ + routeTree: rootRoute.addChildren([signInRoute, dashboardLayout.addChildren([deepRoute])]), + history: router.createMemoryHistory({ initialEntries: ['/org-1/clu-2/apps'] }), + context: { + queryClient: new QueryClient(), + authentication: { [OverallAppSignIn]: { isLoading: false, user: null } }, + }, + }); + + render(); + return instance; +} + +afterEach(() => { + vi.unstubAllEnvs(); + vi.clearAllMocks(); +}); + +describe('Datadog boot view against the real router', () => { + it('starts exactly one view when a signed-out deep link redirects to sign-in', async () => { + const instance = await bootDeepLink(); + + await waitFor(() => expect(instance.state.location.pathname).toBe('/sign-in')); + + expect(rum.startView).toHaveBeenCalledTimes(1); + expect(rum.startView).toHaveBeenCalledWith(expect.objectContaining({ name: '/sign-in/' })); + }); +}); From 7be0de13adadb666360ee06aca4583529cf59273 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Mon, 24 Aug 2026 13:13:27 -0400 Subject: [PATCH 8/9] docs(agents): record which signed-out deep links re-fire the boot view Whether a boot redirect costs the `initial_load` view turns entirely on whether auth is known synchronously, which is not obvious from either file: `beforeLoad` redirects only once `!isLoading && !user`, and `getAllConnections()` reports `isLoading: false` up front unless the `Studio:PotentiallyAuthenticated` record carries an `OverallAppSignIn` entry. Ordinary signed-out deep link: one view. Expired session: two, a round trip apart. Verified against the real router. Refs #1570 Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index c7d3ac8b6..a60bad80d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -436,6 +436,16 @@ Two traps when reading this from RUM data: route actually loaded, and why #1405's "`/` view regression" was really every deep-link entry conflated into one bucket. +A boot-time redirect does **not** cost you that first view, and which path you are on decides it. +`dashboardLayout.beforeLoad` redirects only once auth is known (`!isLoading && !user`), and +`authStore.getAllConnections()` reports `isLoading: false` synchronously unless the +`Studio:PotentiallyAuthenticated` localStorage record holds an `OverallAppSignIn` entry. So an +ordinary signed-out deep link resolves the redirect _during_ the router's initial load and the root +component never commits the deep-link location — one `startView`, named `/sign-in/`. Only a stale +flag (expired session) boots at `isLoading: true`, renders the deep link, and redirects after +`AppRouted`'s `router.invalidate()` — two views, a network round trip apart, the second a genuine +navigation. + Verifying a change here needs a **visible** browser on a production build: a headless or background tab reports `visibilityState: 'hidden'`, emits zero paint and LCP entries, and `trackFirstHidden` discards them anyway — so vitals always read as absent, and a broken fix looks identical to a From e53ca44330a6464d3c94d2f4dd684eb8aa3edc1b Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Mon, 24 Aug 2026 13:21:48 -0400 Subject: [PATCH 9/9] docs(agents): correct which auth-store call decides the boot redirect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit codex (pre-push): the note credited `getAllConnections()` with reporting `isLoading: true` for a persisted session. It does not — that value comes from `getConnectionById`. `getAllConnections()` synthesizes an entry only when the `Studio:PotentiallyAuthenticated` record lacks `OverallAppSignIn`; with the entry present it returns the record untouched and the key is absent, so `beforeLoad` short-circuits on `auth &&` rather than on `isLoading`. Same two outcomes, right mechanism. Also trims the narrating half of the new test's header comment, which both legs flagged. Refs #1570 Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 19 ++++++++++--------- .../datadog/datadogBootView.test.tsx | 5 ++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index a60bad80d..ab6f10b4f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -436,15 +436,16 @@ Two traps when reading this from RUM data: route actually loaded, and why #1405's "`/` view regression" was really every deep-link entry conflated into one bucket. -A boot-time redirect does **not** cost you that first view, and which path you are on decides it. -`dashboardLayout.beforeLoad` redirects only once auth is known (`!isLoading && !user`), and -`authStore.getAllConnections()` reports `isLoading: false` synchronously unless the -`Studio:PotentiallyAuthenticated` localStorage record holds an `OverallAppSignIn` entry. So an -ordinary signed-out deep link resolves the redirect _during_ the router's initial load and the root -component never commits the deep-link location — one `startView`, named `/sign-in/`. Only a stale -flag (expired session) boots at `isLoading: true`, renders the deep link, and redirects after -`AppRouted`'s `router.invalidate()` — two views, a network round trip apart, the second a genuine -navigation. +A boot-time redirect does **not** cost you that first view, and the deciding factor is whether +`getAllConnections()` can answer synchronously. It synthesizes `{ user: null, isLoading: false }` +for `OverallAppSignIn` only when the `Studio:PotentiallyAuthenticated` localStorage record has no +entry for it; with an entry it returns the record untouched, so the key is simply **absent**. +`dashboardLayout.beforeLoad` guards on `auth && !auth.isLoading && !auth.user`, so those are two +different outcomes: a signed-out deep link redirects _during_ the router's initial load and the root +component never commits the deep-link location — one `startView`, named `/sign-in/` — while an +expired session short-circuits on the missing key, renders the deep link, and only redirects once +auth resolves and `AppRouted` calls `router.invalidate()`. Two views there, a network round trip +apart, the second a genuine navigation. Verifying a change here needs a **visible** browser on a production build: a headless or background tab reports `visibilityState: 'hidden'`, emits zero paint and LCP entries, and `trackFirstHidden` diff --git a/src/integrations/datadog/datadogBootView.test.tsx b/src/integrations/datadog/datadogBootView.test.tsx index 0e0185167..129fee457 100644 --- a/src/integrations/datadog/datadogBootView.test.tsx +++ b/src/integrations/datadog/datadogBootView.test.tsx @@ -23,9 +23,8 @@ vi.mock('@/features/notifications/NotificationsSubscriptionManager', () => ({ })); vi.mock('@/components/NotificationBanner', () => ({ NotificationBanner: () => null })); -// The real root route and the real dashboard guard, with stub leaves: the point is that -// `rootRoute`'s component is what mounts the tracker, so this fails if the tracker ever stops -// being mounted at the root — the failure mode the SDK cannot report, because it is "no data". +// Real `rootRoute` and real `dashboardLayout`, stub leaves: mocking either would stop this from +// guarding the one thing it exists for — that the tracker is still mounted at the root. async function bootDeepLink() { vi.resetModules(); vi.stubEnv('DEV', false);