diff --git a/.changeset/brave-pandas-smile.md b/.changeset/brave-pandas-smile.md new file mode 100644 index 00000000000..ee3f77b56c6 --- /dev/null +++ b/.changeset/brave-pandas-smile.md @@ -0,0 +1,5 @@ +--- +'@qwik.dev/router': patch +--- + +fix: preserve matched params in ancestor layout route loaders diff --git a/packages/qwik-router/src/middleware/request-handler/handlers/loader-request-event.unit.ts b/packages/qwik-router/src/middleware/request-handler/handlers/loader-request-event.unit.ts index d3c0bca1122..7b74bca3baa 100644 --- a/packages/qwik-router/src/middleware/request-handler/handlers/loader-request-event.unit.ts +++ b/packages/qwik-router/src/middleware/request-handler/handlers/loader-request-event.unit.ts @@ -96,6 +96,8 @@ describe('createLoaderRequestEventFactory', () => { const routeLoaderCtx = getRouteLoaderCtx(requestEv); routeLoaderCtx.loaderPaths['products-loader'] = '/products/'; routeLoaderCtx.loaderPaths['details-loader'] = '/products/123/'; + routeLoaderCtx.loaderParams['products-loader'] = {}; + routeLoaderCtx.loaderParams['details-loader'] = { id: '123' }; const getLoaderRequestEvent = createLoaderRequestEventFactory(requestEv); const productsLoader = createLoader('products-loader', ['page']); const detailsLoader = createLoader('details-loader', ['page']); @@ -111,6 +113,7 @@ describe('createLoaderRequestEventFactory', () => { expect(productsEv.request.url).toBe('http://localhost/products/?page=2'); expect(productsEv.originalUrl.href).toBe('http://localhost/products/?page=2'); expect(productsEv.params).toEqual({}); + expect(detailsEv.params).toEqual({ id: '123' }); } finally { globalThis.__STRICT_LOADERS__ = previousStrictLoaders; } diff --git a/packages/qwik-router/src/middleware/request-handler/resolve-request-handlers-core.ts b/packages/qwik-router/src/middleware/request-handler/resolve-request-handlers-core.ts index e9ec3aa08d7..2331e36938b 100644 --- a/packages/qwik-router/src/middleware/request-handler/resolve-request-handlers-core.ts +++ b/packages/qwik-router/src/middleware/request-handler/resolve-request-handlers-core.ts @@ -363,6 +363,9 @@ function createResolveRequestHandlers() { if (route.$loaderPaths$) { Object.assign(routeLoaderCtx.loaderPaths, route.$loaderPaths$); } + if (route.$loaderParams$) { + Object.assign(routeLoaderCtx.loaderParams, route.$loaderParams$); + } // Store loader internals so SSG can check __cacheControl. setRouteLoaders(requestEv, routeLoaders); diff --git a/packages/qwik-router/src/middleware/request-handler/resolve-request-handlers.unit.ts b/packages/qwik-router/src/middleware/request-handler/resolve-request-handlers.unit.ts index de695bf3da9..62e4cad56dc 100644 --- a/packages/qwik-router/src/middleware/request-handler/resolve-request-handlers.unit.ts +++ b/packages/qwik-router/src/middleware/request-handler/resolve-request-handlers.unit.ts @@ -498,6 +498,37 @@ describe('resolve-request-handler', () => { expect(background).toHaveBeenCalledTimes(1); }); + it('scopes ancestor loader params to their matched path', async () => { + const previousStrictLoaders = globalThis.__STRICT_LOADERS__; + globalThis.__STRICT_LOADERS__ = true; + try { + const layoutLoader = vi.fn(() => 'layout'); + const route = pageRouteWithLoaders(makeLoader('layout-loader', layoutLoader)); + Object.assign(route, { + $routeName$: '/[tenantSlug]/agents/', + $params$: { tenantSlug: 'acme' }, + $loaderPaths$: { 'layout-loader': '/acme/' }, + $loaderParams$: { 'layout-loader': { tenantSlug: 'acme' } }, + }); + const handlers = resolveRequestHandlers(undefined, route, 'GET', true, exitRender()); + const requestEv = createRequestEvent( + createMockServerRequestEvent('http://localhost:3000/acme/agents/'), + route, + handlers, + '/', + vi.fn() + ); + + await requestEv.next(); + + expect(layoutLoader).toHaveBeenCalledWith( + expect.objectContaining({ params: { tenantSlug: 'acme' } }) + ); + } finally { + globalThis.__STRICT_LOADERS__ = previousStrictLoaders; + } + }); + it('does not await blockSSR:false loaders before render', async () => { let release!: () => void; const gate = new Promise((resolve) => (release = () => resolve('late'))); diff --git a/packages/qwik-router/src/runtime/src/route-loaders.ts b/packages/qwik-router/src/runtime/src/route-loaders.ts index 8685a80fd7b..1207d0d2609 100644 --- a/packages/qwik-router/src/runtime/src/route-loaders.ts +++ b/packages/qwik-router/src/runtime/src/route-loaders.ts @@ -41,6 +41,7 @@ import type { LoaderConstructorQRL, LoaderInternal, LoaderOptions, + PathParams, RequestEvent, RequestEventLoader, RouteNavigate, @@ -118,12 +119,14 @@ const wrapWithAbort = (promise: Promise, signal: AbortSignal): Promise * a store that gets updated on navigation. * * - `loaderPaths`: loader ID → fetch path (the longest route path for that loader) + * - `loaderParams`: loader ID → params resolved at the loader path * - `pagePathname` / `pageSearch`: client-only navigation state used for loader invalidation and * q-loader fetches. They are intentionally omitted from SSR state and fall back to `location` * until the first SPA navigation. */ export type RouteLoaderCtx = { loaderPaths: Record; + loaderParams: Record; pagePathname?: string; pageSearch?: string; /** SPA navigation function. Client-only and intentionally omitted from SSR state. */ @@ -555,6 +558,7 @@ export function getRouteLoaderCtx(requestEv: RequestEventBase): RouteLoaderCtx { if (!ctx) { ctx = { loaderPaths: {}, + loaderParams: {}, }; requestEv.sharedMap.set(REQUEST_LOADER_PATHS_STORE, ctx); } @@ -772,9 +776,12 @@ export const getLoaderRequestEvent = ( } const url = new URL(rootRequestEv.url); + const routeLoaderCtx = getRouteLoaderCtx(rootRequestEv); const pathname = globalThis.__STRICT_LOADERS__ - ? getRouteLoaderCtx(rootRequestEv).loaderPaths[loader.__id] || rootRequestEv.url.pathname + ? routeLoaderCtx.loaderPaths[loader.__id] || rootRequestEv.url.pathname : rootRequestEv.url.pathname; + const scopedParams = + pathname === rootRequestEv.url.pathname ? {} : routeLoaderCtx.loaderParams[loader.__id] || {}; const filteredSearch = loader.__search ? filterSearchParams(url.searchParams, loader.__search) : rootRequestEv.url.search; @@ -800,7 +807,7 @@ export const getLoaderRequestEvent = ( enumerable: true, }, params: { - value: {}, + value: scopedParams, enumerable: true, }, pathname: { diff --git a/packages/qwik-router/src/runtime/src/route-loaders.unit.ts b/packages/qwik-router/src/runtime/src/route-loaders.unit.ts index 62ca50c9995..db566a4818d 100644 --- a/packages/qwik-router/src/runtime/src/route-loaders.unit.ts +++ b/packages/qwik-router/src/runtime/src/route-loaders.unit.ts @@ -15,7 +15,7 @@ import type { LoaderInternal } from './types'; describe('route loader execution', () => { it('stores an uninitialized resume marker for never loaders', () => { const state = {} as RouteLoaderState; - const routeLoaderCtx = { loaderPaths: {} }; + const routeLoaderCtx = { loaderPaths: {}, loaderParams: {} }; const neverLoader = createLoader('never-loader', async () => undefined); const alwaysLoader = createLoader('always-loader', async () => undefined, 'always'); @@ -31,7 +31,7 @@ describe('route loader execution', () => { it('registers immutable loaders so nav-wide invalidation skips them', () => { const state = {} as RouteLoaderState; - const routeLoaderCtx = { loaderPaths: {} }; + const routeLoaderCtx = { loaderPaths: {}, loaderParams: {} }; const immutable = routeLoaderQrl(createQrl('immutable-loader'), { cacheControl: 'immutable', }) as LoaderInternal; @@ -46,7 +46,7 @@ describe('route loader execution', () => { it('invalidates loader signals on nav, skipping resumed values and immutable loaders', () => { const state = {} as RouteLoaderState; - const routeLoaderCtx = { loaderPaths: {} }; + const routeLoaderCtx = { loaderPaths: {}, loaderParams: {} }; const immutable = routeLoaderQrl(createQrl('nav-immutable-loader'), { cacheControl: 'immutable', }) as LoaderInternal; diff --git a/packages/qwik-router/src/runtime/src/routing.ts b/packages/qwik-router/src/runtime/src/routing.ts index 0d1e38ff1c6..b5228a2df50 100644 --- a/packages/qwik-router/src/runtime/src/routing.ts +++ b/packages/qwik-router/src/runtime/src/routing.ts @@ -31,6 +31,7 @@ export const loadRoute = async ( routeBundleNames, loaderHashes, loaderPathsByHash, + loaderParamsByHash, menuLoader, errorLoader, } = result; @@ -72,6 +73,7 @@ export const loadRoute = async ( $errorLoader$: errorLoader, $loaders$: loaderHashes, $loaderPaths$: loaderPathsByHash, + $loaderParams$: loaderParamsByHash, }; }; @@ -206,7 +208,9 @@ function collectNodeMeta( menuLoaderRef: { v: MenuModuleLoader | undefined }, loaderHashes?: string[], loaderPathsByHash?: Record, - matchedPathname = '/' + matchedPathname = '/', + loaderParamsByHash?: Record, + matchedParams: PathParams = {} ) { for (let j = 0; j < groups.length; j++) { const g = groups[j]; @@ -219,6 +223,9 @@ function collectNodeMeta( for (let i = 0; i < g._R.length; i++) { const hash = g._R[i]; loaderPathsByHash[hash] = matchedPathname; + if (loaderParamsByHash) { + loaderParamsByHash[hash] = matchedParams; + } } } } @@ -243,6 +250,9 @@ function collectNodeMeta( for (let i = 0; i < node._R.length; i++) { const hash = node._R[i]; loaderPathsByHash[hash] = matchedPathname; + if (loaderParamsByHash) { + loaderParamsByHash[hash] = matchedParams; + } } } } @@ -477,6 +487,7 @@ function matchRouteTree( routeBundleNames: string[] | undefined; loaderHashes: string[] | undefined; loaderPathsByHash: Record | undefined; + loaderParamsByHash: Record | undefined; menuLoader: MenuModuleLoader | undefined; /** The nearest _E (error.tsx) boundary's chain to render on a thrown error (in its layouts). */ errorLoader: ModuleLoader[] | undefined; @@ -487,6 +498,7 @@ function matchRouteTree( const layouts: ModuleLoader[] = []; const loaderHashes: string[] = []; const loaderPathsByHash: Record = {}; + const loaderParamsByHash: Record = {}; const errorLoaderRef: BoundaryRef = { v: undefined, layouts: [] }; const notFoundLoaderRef: BoundaryRef = { v: undefined, layouts: [] }; const menuLoaderRef: { v: MenuModuleLoader | undefined } = { v: undefined }; @@ -503,7 +515,9 @@ function matchRouteTree( notFoundLoaderRef, menuLoaderRef, loaderHashes, - loaderPathsByHash + loaderPathsByHash, + '/', + loaderParamsByHash ); if (root._M) { groupNodes.push({ node: root, depth: layouts.length }); @@ -528,6 +542,7 @@ function matchRouteTree( params: PathParams; layouts: ModuleLoader[]; loaderPathsByHash: Record; + loaderParamsByHash: Record; errorLoader: ContentModuleLoader | ModuleLoader[] | undefined; errorLayouts: ModuleLoader[]; notFoundLoader: ContentModuleLoader | ModuleLoader[] | undefined; @@ -561,6 +576,7 @@ function matchRouteTree( params: { ...params }, layouts: [...layouts], loaderPathsByHash: { ...loaderPathsByHash }, + loaderParamsByHash: { ...loaderParamsByHash }, errorLoader: errorLoaderRef.v, errorLayouts: errorLoaderRef.layouts, notFoundLoader: notFoundLoaderRef.v, @@ -586,7 +602,9 @@ function matchRouteTree( menuLoaderRef, loaderHashes, loaderPathsByHash, - matchedPathname + matchedPathname, + loaderParamsByHash, + { ...params } ); if (node._M) { groupNodes.push({ node, depth: layouts.length }); @@ -611,7 +629,9 @@ function matchRouteTree( menuLoaderRef, loaderHashes, loaderPathsByHash, - pathname + pathname, + loaderParamsByHash, + { ...params } ); node = indexResult.target; } @@ -635,7 +655,9 @@ function matchRouteTree( menuLoaderRef, loaderHashes, loaderPathsByHash, - pathname + pathname, + loaderParamsByHash, + { ...params } ); node = next; } @@ -654,6 +676,7 @@ function matchRouteTree( const fbRouteParts = [...fb.routeParts, `[...${fb.paramName}]`]; const fbLayouts = [...fb.layouts]; const fbLoaderPathsByHash = { ...fb.loaderPathsByHash }; + const fbLoaderParamsByHash = { ...fb.loaderParamsByHash }; const fbErrorRef: BoundaryRef = { v: fb.errorLoader, layouts: fb.errorLayouts }; const fbNotFoundRef: BoundaryRef = { v: fb.notFoundLoader, layouts: fb.notFoundLayouts }; const fbMenuRef: { v: MenuModuleLoader | undefined } = { v: fb.menuLoader }; @@ -668,7 +691,9 @@ function matchRouteTree( fbMenuRef, fbLoaderHashes, fbLoaderPathsByHash, - pathname + pathname, + fbLoaderParamsByHash, + fbParams ); const fbLoaders = resolveLoaders(root, fb.aNode, fbLayouts); @@ -682,6 +707,8 @@ function matchRouteTree( loaderHashes: fbLoaderHashes.length > 0 ? fbLoaderHashes : undefined, loaderPathsByHash: Object.keys(fbLoaderPathsByHash).length > 0 ? fbLoaderPathsByHash : undefined, + loaderParamsByHash: + Object.keys(fbLoaderParamsByHash).length > 0 ? fbLoaderParamsByHash : undefined, menuLoader: fbMenuRef.v, errorLoader: boundaryChain(fbErrorRef), }; @@ -737,6 +764,7 @@ function matchRouteTree( routeBundleNames: undefined, loaderHashes: undefined, loaderPathsByHash: undefined, + loaderParamsByHash: undefined, menuLoader: menuLoaderRef.v, errorLoader: boundaryChain(errorLoaderRef), }; @@ -749,6 +777,7 @@ function matchRouteTree( for (let i = 0; i < node._R.length; i++) { const hash = node._R[i]; loaderPathsByHash[hash] = matchedPathname; + loaderParamsByHash[hash] = { ...params }; } } @@ -760,6 +789,7 @@ function matchRouteTree( routeBundleNames: node._B as string[] | undefined, loaderHashes: loaderHashes.length > 0 ? loaderHashes : undefined, loaderPathsByHash: Object.keys(loaderPathsByHash).length > 0 ? loaderPathsByHash : undefined, + loaderParamsByHash: Object.keys(loaderParamsByHash).length > 0 ? loaderParamsByHash : undefined, menuLoader: menuLoaderRef.v, errorLoader: boundaryChain(errorLoaderRef), }; diff --git a/packages/qwik-router/src/runtime/src/routing.unit.ts b/packages/qwik-router/src/runtime/src/routing.unit.ts index 0c50bd3a750..2a7b03c5a68 100644 --- a/packages/qwik-router/src/runtime/src/routing.unit.ts +++ b/packages/qwik-router/src/runtime/src/routing.unit.ts @@ -247,6 +247,36 @@ test('loadRoute — loader paths are replaced by deeper matches', async () => { }); }); +test('loadRoute — loader params match each loader path', async () => { + const routes: RouteData = { + _R: ['root-loader'], + _W: { + _P: 'tenantSlug', + _R: ['tenant-loader'], + agents: { + _W: { + _P: 'agentId', + _R: ['agent-loader'], + view: { + _R: ['page-loader'], + _I: makeLoader(), + }, + }, + }, + }, + }; + + const result = await loadRoute(routes, false, '/acme/agents/42/view'); + + assert.isFalse(result.$notFound$); + assert.deepEqual(result.$loaderParams$, { + 'root-loader': {}, + 'tenant-loader': { tenantSlug: 'acme' }, + 'agent-loader': { tenantSlug: 'acme', agentId: '42' }, + 'page-loader': { tenantSlug: 'acme', agentId: '42' }, + }); +}); + test('loadRoute — miss renders the nearest _4 inside gathered layouts', async () => { const rootLayout = { default: () => 'layout' }; const notFound = { default: () => 'not-found' }; diff --git a/packages/qwik-router/src/runtime/src/types.ts b/packages/qwik-router/src/runtime/src/types.ts index 2a033d86a3d..591caf14bc6 100644 --- a/packages/qwik-router/src/runtime/src/types.ts +++ b/packages/qwik-router/src/runtime/src/types.ts @@ -446,6 +446,8 @@ export interface LoadedRoute { $loaders$?: string[]; /** Runtime-only mapping of routeLoader$ hashes to the matched pathname used for q-loader fetches */ $loaderPaths$?: Record; + /** Runtime-only mapping of routeLoader$ hashes to params matched at their loader path */ + $loaderParams$?: Record; } export interface EndpointResponse {