Repository navigation
router: fixes and improvements for the router inspector #349
Copy link
Copy link
Closed
Labels
bugSomething is broken or shows wrong dataSomething is broken or shows wrong datafeatureA feature request or a pull request that adds oneA feature request or a pull request that adds one
Description
Context
Fixes and improvements from a code audit of the router inspector, done in October 2026. Roadmap work lives in #340, #341, #342 and #343. Items marked (to confirm) were not reproduced; check them before fixing.
Problems
Medium
- instrument and resolve-lazy ignore
actions.router:ROUTER_WRITE_ACTIONSlists only navigate, abort, replay and probe.instrumentrewrites guard and resolver entries in the live config, andresolve-lazyruns the app'sloadChildrenimport with its side effects, so both still run with router writes off, whileprobe(which may also load lazy chunks) is blocked. Decide whether they count as writes and gate them, or document why they are safe. Splitting a read-only probe tool from the writenavigatetool would letACTION_TOOLS.routerlist the write one (packages/devtools/src/config.ts:404,packages/devtools/src/devframe.ts:2323). - links on parents are missed:
linksOfonly queriesa, button, [routerlink], [routerLink]. It misses<li routerLinkActive><a routerLink>, whereRouterLinkActivesits on a parent, and property-bound[routerLink]on other elements, which leave no attribute. Thelink-aria-currentlint and the link checks ininspect-routeskip these links. Also query[routerlinkactive], or find the directives throughng.getDirectiveson hosts the components collector knows, and report a truncated count whenMAX_LINKS(100) is hit (packages/devtools/src/router-links.ts:164). - router attached only once: the overlay finds the Router once and never checks it again. After an HMR re-bootstrap, a late micro-frontend or Angular Elements, the old router stays watched and
routerCleanupnever runs. After 3 pushes that find a component but no router, it stops looking for good (routerMisses < 3). Check on a backoff that the router is still reachable from a live root; when it is gone, runrouterCleanup, reset state and attach again. Replace the 3-miss cap withcreateScanBackoff(packages/devtools/src/overlay.ts:408-432).
Low
- shared module state across routers:
pendingCaller,pendingRedirect,configGenerationandknownSecretsare module globals, so with several routers a redirect or caller from one can attach to another'sNavigationStart, and a DevToolsnavigate()racing an app navigation can be mislabelled. Move them into a per-router context returned bywatchRouter(packages/devtools/src/router.ts:165-172). - title set after the push:
nav.titleis written in a microtask afteronChangealready ran, so no push is scheduled and the title only shows up on a later push. CallonChangeorscheduleRouterPushafter setting it, or read the title lazily on the next push (packages/devtools/src/router.ts:1042-1045). - instrumentation described as opt-in:
storedInstrumented()returnstrueunless sessionStorage holds'0', so it is on by default, but the JSDoc says "Opt-in" and theexplain-navigationheader always tells agents to "Turn instrumentation on". Fix the JSDoc and give the instruction only whenpage.instrumentedis false (packages/devtools/src/router-actions.ts:91-96,541-545;packages/devtools/src/rpc/router-tools.ts:813). - explain-navigation capped at 50:
limitis clamped to the hard-codedMAX_NAVIGATIONS = 50, while the hub keeps up tolimits.navigationsper page. Passlimits.navigationsintoexplainNavigationText(packages/devtools/src/rpc/router-tools.ts:51,797;packages/devtools/src/devframe.ts:501). - empty-redirect-prefix label is wrong: the finding says
angular: 'throws'. Angular throws only whenpathMatchis missing, and then the app fails before it reports its config. The rule can only fire for an explicitpathMatch: 'prefix', where Angular stays silent. Setangular: 'silent'and say that an explicit'prefix'on an empty-path redirect matches every URL (packages/devtools/src/rpc/router-config-tools.ts:384-395). - param-shadows-literal false positives: a
:paramroute with children and no empty-path child does not shadow a later literal sibling, because the router backtracks when the children fail to match. Flag only leaf param routes or ones with an empty-path child, or reusematchUrlto confirm (packages/devtools/src/rpc/router-config-tools.ts:367-383). - probe uses magic numbers and waits on a sync throw:
probe()compares bare event types (4,2,1,3,16) and cancel code0instead ofEventTypeandCANCEL_CODES. WhennavigateByUrlthrows synchronously,promiseis null and the probe waits the full 10s. Use the constants and callfinish({ error })at once whenpromiseis null (packages/devtools/src/router-actions.ts:703-711,746-754). - panel copies server types by hand:
app/src/pages/router-types.tsredefinesNavigationRecordandRouteNode(missingdataKeys,inputs,classGuards,runGuardsAndResolvers),isReplayableUrlcopiesisSafeUrl, andguardResultalready differs ('running'vs'still running'). Export the types and pure helpers from a shared types-only entry point, or add a contract test (app/src/pages/router-types.ts:81-97,207;app/src/pages/route-timeline.ts:696;packages/devtools/src/rpc/router-tools.ts:602-607;packages/devtools/src/rpc/router-loops.ts:345). - timeline phase colours are hard-coded:
#60a5fa,#f59e0b,#2dd4bfand#34d399are not theme tokens. Amber and teal onvar(--surface-3)in the light theme probably fall below the 3:1 non-text contrast of WCAG 1.4.11. Add--phase-*tokens per theme followingui-guidelines.mdand check both themes (app/src/pages/route-timeline.ts:20-25) (to confirm). - route nesting is visual only: the active-routes table shows depth only through
padding-left, so screen readers do not hear the hierarchy. Add visually hidden depth text ("level 2, child of /users") or use a treegrid witharia-level(app/src/pages/route-current.ts:97). - in-place config edits not detected:
configSignaturecounts only routes and loaded routes, so a path or guard changed with the same count does not bump the generation, and the cached config, instrumentation and lint stay stale. Add a cheap hash of path,redirectToand guard counts per node, still capped atMAX_NODES(packages/devtools/src/router-config.ts:214-236). - events-only wording contradicts the data:
modeisevents-onlyonly whenɵgetInjectorProvidersis missing, butpushRouterstill walks and sends the config whenever a router andngexist. The messages say the config cannot be read. Tie the wording to what is actually missing (setup detection, preloader and scroller lookup), or set the mode from whether the config could be walked (packages/devtools/src/router-setup.ts:241;packages/devtools/src/rpc/router-config-tools.ts:234,580,614). - skipped navigations marked imperative: a
NavigationSkippedwithout aNavigationStartis always recorded with trigger'imperative', even for popstate or hashchange, so the loop and burst heuristics treat it as code-started. Use'unknown'or the trigger fromcurrentNavigation, and leave these out ofisAutomatic(packages/devtools/src/router.ts:965-977).
Improvements
- One "what happens if I go to X" result: the matched chain, the effective guards per level from the protection audit, the render mode from
explain-render-modeand the source file in one card (packages/devtools/src/rpc/router-config-tools.ts,app/src/pages/route-*.ts). - A "diff two navigations" view on top of replay: which guard verdicts, resolvers, params and phase timings changed (
packages/devtools/src/router-actions.ts,app/src/pages/route-timeline.ts). - Turn the
redirect-from-querylint into a live check: when instrumentation sees a guard return aUrlTreebuilt from a query param, flag a possible open redirect with the guard name (packages/devtools/src/router-actions.ts,packages/devtools/src/rpc/router-config-tools.ts). - Take the per-navigation HTTP request list from the HTTP collector instead of
PerformanceResourceTiming, for status codes and sizes without unrelated concurrent requests (packages/devtools/src/router.ts,packages/devtools/src/rpc/router-tools.ts). - An MCP resource or tool that streams new navigations, so agents can follow a login and redirect flow without polling
explain-navigation(packages/devtools/src/devframe.ts,packages/devtools/src/config.ts). - Record whether
withNavigationErrorHandleror the platform Navigation API is in use, and show the browser Navigation API entries next to router history to explainurlDrift(packages/devtools/src/router-setup.ts,app/src/pages/route-current.ts). - Lint rules for
runGuardsAndResolvers: 'always'on busy parents, resolvers on routes whose components fetch the same data, andloadComponentroutes whose component is already in the eager bundle (packages/devtools/src/rpc/router-config-tools.ts).
Already tracked elsewhere
ACTION_TOOLS.routeris empty, soactions.router: falsedoes not hidenavigatefrom agents: phase 1 of 4: safe by default and wired into your workflow #340.- Open in editor from route tree rows: phase 1 of 4: safe by default and wired into your workflow #340.
- Router events as a layer on one shared timeline: phase 3 of 4: a shell you live in #342.
Generated by Claude Code
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
bugSomething is broken or shows wrong dataSomething is broken or shows wrong datafeatureA feature request or a pull request that adds oneA feature request or a pull request that adds one
Activity
fix: audit findings for store, forms, router and Analog (#362)