From 259632765ded372b3f0c355daad3ac85f69ccf64 Mon Sep 17 00:00:00 2001 From: Brenley Dueck Date: Mon, 6 Jul 2026 16:23:19 -0500 Subject: [PATCH] fix: params becoming undefined in outgoing components during navigation Solid 2 disposes roots created inside a computation when it re-runs, so route roots created in the routeStates memo were disposed on every navigation, killing reactive nodes they owned. Roots are now created under the Routes component owner. Route contexts also get params scoped to their own lifetime via a sticky matches memo that retains its last valid value while the route is torn down. Components, effects, and preloads in outgoing routes (e.g. kept alive by a Loading boundary) no longer observe another route's params or refetch with undefined values. Also removes the routeMatches[0] fallback that could hand a context the wrong match. --- .changeset/fix-stale-params-on-navigation.md | 7 + src/routers/components.tsx | 37 ++- src/routing.ts | 39 +-- src/types.ts | 2 + test/client-navigation.spec.tsx | 242 ++++++++++++++++++- 5 files changed, 297 insertions(+), 30 deletions(-) create mode 100644 .changeset/fix-stale-params-on-navigation.md diff --git a/.changeset/fix-stale-params-on-navigation.md b/.changeset/fix-stale-params-on-navigation.md new file mode 100644 index 00000000..a1198e34 --- /dev/null +++ b/.changeset/fix-stale-params-on-navigation.md @@ -0,0 +1,7 @@ +--- +"@solidjs/router": patch +--- + +Fix params becoming undefined in outgoing components during navigation. + +Route roots are now created under the `Routes` component owner so they survive re-runs of the route state memo (Solid 2 disposes roots created inside a computation when it re-runs). Each route context also gets params scoped to its own lifetime: they retain their last values while the route is being torn down, so effects and async fetches in outgoing components (e.g. kept alive by a `` boundary) never observe another route's params. This also removes the incorrect fallback that could hand a route context another route's match during teardown. diff --git a/src/routers/components.tsx b/src/routers/components.tsx index 40e82e7d..af559498 100644 --- a/src/routers/components.tsx +++ b/src/routers/components.tsx @@ -1,7 +1,7 @@ /*@refresh skip*/ import type {Component, JSX, Owner} from "solid-js"; -import {children, createMemo, createRoot, getOwner, merge, untrack} from "solid-js"; +import {children, createMemo, createRoot, getOwner, merge, runWithOwner, untrack} from "solid-js"; import {getRequestEvent, isServer, type RequestEvent} from "@solidjs/web"; import { createBranches, @@ -117,6 +117,10 @@ function Routes(props: { routerState: RouterContext; branches: Branch[] }) { const disposers: (() => void)[] = []; let root: RouteContext | undefined; let prevMatches: RouteMatch[] | undefined; + // Route roots must outlive re-runs of the `routeStates` memo below, so they + // are created under the owner of this component rather than the memo's + // computation (which disposes its children every time it re-runs). + const owner = getOwner()!; const routeStates = createMemo((prev: RouteContext[] | undefined) => { const nextMatches = props.routerState.matches(); @@ -135,18 +139,27 @@ function Routes(props: { routerState: RouterContext; branches: Branch[] }) { disposers[i](); } - createRoot(dispose => { - disposers[i] = dispose; - next[i] = createRouteContext( - props.routerState, - next[i - 1] || props.routerState.base, - createOutlet(() => routeStates()?.[i + 1]), - () => { + runWithOwner(owner, () => + createRoot(dispose => { + disposers[i] = dispose; + const routeKey = nextMatch.route.key; + // Retain the last matches in which this route participated so + // that its components and preloads never observe another + // route's params/path while this route is being torn down. + const matchesAtLevel = createMemo((prev?: RouteMatch[]) => { const routeMatches = props.routerState.matches(); - return routeMatches[i] ?? routeMatches[0]; - } - ); - }); + const m = routeMatches[i]; + return m && m.route.key === routeKey ? routeMatches : prev || nextMatches; + }); + next[i] = createRouteContext( + props.routerState, + next[i - 1] || props.routerState.base, + createOutlet(() => routeStates()?.[i + 1]), + () => matchesAtLevel()[i], + matchesAtLevel + ); + }) + ); } } diff --git a/src/routing.ts b/src/routing.ts index 06e659e6..043e90f4 100644 --- a/src/routing.ts +++ b/src/routing.ts @@ -196,7 +196,7 @@ export const useCurrentMatches = () => useRouter().matches; * const getUser = query(() => fetchUser(params.id), "user"); * ``` */ -export const useParams = () => useRouter().params as T; +export const useParams = () => useRoute().params as T; /** * Retrieves a tuple containing a reactive object to read the current location's query parameters and a method to update them. @@ -378,6 +378,14 @@ export function getRouteMatches(branches: Branch[], location: string): RouteMatc return []; } +function mergeParams(matches: RouteMatch[]): Params { + const params: Params = {}; + for (let i = 0; i < matches.length; i++) { + Object.assign(params, matches[i].params); + } + return params; +} + function createLocation( path: Accessor, state: Accessor, @@ -487,21 +495,17 @@ export function createRouterContext( return getRouteMatches(branches(), location.pathname); }); - const buildParams = () => { - const m = matches(); - const params: Params = {}; - for (let i = 0; i < m.length; i++) { - Object.assign(params, m[i].params); - } - return params; - }; + const buildParams = () => mergeParams(matches()); + + const wrapParams = utils.paramsWrapper + ? (getParams: () => Params) => utils.paramsWrapper!(getParams, branches) + : (getParams: () => Params) => createMemoObject(getParams); - const params = utils.paramsWrapper - ? utils.paramsWrapper(buildParams, branches) - : createMemoObject(buildParams); + const params = wrapParams(buildParams); const baseRoute: RouteContext = { pattern: basePath, + params, path: () => basePath, outlet: () => null, resolvePath(to: string) { @@ -513,6 +517,7 @@ export function createRouterContext( base: baseRoute, location, params, + wrapParams, isRouting, renderPath, parsePath, @@ -673,11 +678,16 @@ export function createRouteContext( router: RouterContext, parent: RouteContext, outlet: () => JSX.Element, - match: () => RouteMatch + match: () => RouteMatch, + matches: () => RouteMatch[] = () => [match()] ): RouteContext { - const { base, location, params } = router; + const { base, location, wrapParams } = router; const { pattern, component, preload } = match().route; const path = createMemo(() => match().path); + // Params scoped to this route's lifetime. `matches` is expected to retain + // its last valid value while this route is being torn down, so outgoing + // components and preloads never observe another route's params. + const params = wrapParams(() => mergeParams(matches())); component && (component as MaybePreloadableComponent).preload && @@ -689,6 +699,7 @@ export function createRouteContext( const route: RouteContext = { parent, pattern, + params, path, outlet: () => component diff --git a/src/types.ts b/src/types.ts index 9b8d1ab2..bb30eece 100644 --- a/src/types.ts +++ b/src/types.ts @@ -155,6 +155,7 @@ export interface RouteContext { parent?: RouteContext; child?: RouteContext; pattern: string; + params: Params; path: () => string; outlet: () => JSX.Element; resolvePath(to: string): string | undefined; @@ -173,6 +174,7 @@ export interface RouterContext { base: RouteContext; location: Location; params: Params; + wrapParams: (getParams: () => Params) => Params; navigatorFactory: NavigatorFactory; isRouting: () => boolean; matches: () => RouteMatch[]; diff --git a/test/client-navigation.spec.tsx b/test/client-navigation.spec.tsx index 807e9f1a..cb4c39ea 100644 --- a/test/client-navigation.spec.tsx +++ b/test/client-navigation.spec.tsx @@ -1,11 +1,31 @@ import { render } from "@solidjs/web"; +import { createEffect, createMemo, Loading } from "solid-js"; import { vi } from "vitest"; -import { A, MemoryRouter, Route } from "../src/index.js"; +import { + A, + MemoryRouter, + Route, + createMemoryHistory, + useNavigate, + useParams, + type Navigator +} from "../src/index.js"; + +const settle = async (ms = 0) => { + await new Promise(resolve => queueMicrotask(() => resolve())); + await new Promise(resolve => setTimeout(resolve, ms)); +}; describe("Client navigation should", () => { - test("update rendered routes after clicking links", async () => { - const originalScrollTo = window.scrollTo; + const originalScrollTo = window.scrollTo; + beforeEach(() => { window.scrollTo = vi.fn(); + }); + afterAll(() => { + window.scrollTo = originalScrollTo; + }); + + test("update rendered routes after clicking links", async () => { const div = document.createElement("div"); document.body.appendChild(div); @@ -52,7 +72,221 @@ describe("Client navigation should", () => { } finally { dispose(); div.remove(); - window.scrollTo = originalScrollTo; + } + }); + + test("keep params stable in outgoing components when navigating away", async () => { + const div = document.createElement("div"); + document.body.appendChild(div); + + const observed: (string | undefined)[][] = []; + let navigate!: Navigator; + + const history = createMemoryHistory(); + history.set({ value: "/users/1/comments" }); + + const Users = () => { + const params = useParams(); + navigate = useNavigate(); + createEffect( + () => [params.id, params.view] as const, + ([id, view]) => { + observed.push([id, view]); + } + ); + return
{params.id}
; + }; + + const dispose = render( + () => ( + + +
About page
} /> + + ), + div + ); + + try { + await settle(); + expect(div.querySelector('[data-route="users"]')?.textContent).toBe("1"); + expect(observed).toEqual([["1", "comments"]]); + + // Same-route param navigation should update params. + navigate("/users/2/likes"); + await settle(); + expect(div.querySelector('[data-route="users"]')?.textContent).toBe("2"); + expect(observed).toEqual([ + ["1", "comments"], + ["2", "likes"] + ]); + + // Navigating away must not re-run the outgoing route's effects with + // another route's (empty) params. + navigate("/about"); + await settle(); + expect(div.querySelector('[data-route="about"]')?.textContent).toBe("About page"); + expect(div.querySelector('[data-route="users"]')).toBeNull(); + expect(observed).toEqual([ + ["1", "comments"], + ["2", "likes"] + ]); + } finally { + dispose(); + div.remove(); + } + }); + + test("not refetch with empty params in outgoing components suspended by a Loading boundary", async () => { + const div = document.createElement("div"); + document.body.appendChild(div); + + const fetched: (string | undefined)[][] = []; + let navigate!: Navigator; + + const history = createMemoryHistory(); + history.set({ value: "/users/1/comments" }); + + const Users = () => { + const params = useParams(); + navigate = useNavigate(); + const data = createMemo(async () => { + fetched.push([params.id, params.view]); + return `${params.id}-${params.view}`; + }); + return
{data()}
; + }; + + const About = () => { + const data = createMemo(async () => { + await new Promise(resolve => setTimeout(resolve, 25)); + return "About page"; + }); + return
{data()}
; + }; + + const dispose = render( + () => ( + ( + loading}>{props.children} + )} + > + + + + ), + div + ); + + try { + await settle(10); + expect(div.querySelector('[data-route="users"]')?.textContent).toBe("1-comments"); + expect(fetched).toEqual([["1", "comments"]]); + + // Navigating to a suspending route keeps the outgoing tree alive while + // the new one settles; it must not refetch with the new route's params. + navigate("/about"); + await settle(50); + expect(div.querySelector('[data-route="about"]')?.textContent).toBe("About page"); + expect(fetched).toEqual([["1", "comments"]]); + } finally { + dispose(); + div.remove(); + } + }); + + test("expose child params to parent layouts", async () => { + const div = document.createElement("div"); + document.body.appendChild(div); + + let navigate!: Navigator; + + const history = createMemoryHistory(); + history.set({ value: "/users/1" }); + + const Layout = (props: { children?: any }) => { + const params = useParams(); + navigate = useNavigate(); + return ( + <> +
{params.id ?? "none"}
+ {props.children} + + ); + }; + + const dispose = render( + () => ( + + +
Index
} /> +
User
} /> + + + ), + div + ); + + try { + await settle(); + expect(div.querySelector("[data-layout-id]")?.textContent).toBe("1"); + + navigate("/users/2"); + await settle(); + expect(div.querySelector("[data-layout-id]")?.textContent).toBe("2"); + + navigate("/users"); + await settle(); + expect(div.querySelector('[data-route="index"]')).toBeTruthy(); + expect(div.querySelector("[data-layout-id]")?.textContent).toBe("none"); + } finally { + dispose(); + div.remove(); + } + }); + + test("call preload with the route's own params during navigation", async () => { + const div = document.createElement("div"); + document.body.appendChild(div); + + const preloadedParams: (string | undefined)[] = []; + let navigate!: Navigator; + + const history = createMemoryHistory(); + history.set({ value: "/users/1" }); + + const Home = () => { + navigate = useNavigate(); + return
Users
; + }; + + const dispose = render( + () => ( + + + { + preloadedParams.push(params.postId); + }} + component={() =>
Post
} + /> + + ), + div + ); + + try { + await settle(); + navigate("/posts/42"); + await settle(); + expect(div.querySelector('[data-route="post"]')).toBeTruthy(); + expect(preloadedParams).toEqual(["42"]); + } finally { + dispose(); + div.remove(); } }); });