Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/brave-pandas-smile.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@qwik.dev/router': patch
---

fix: preserve matched params in ancestor layout route loaders
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>((resolve) => (release = () => resolve('late')));
Expand Down
11 changes: 9 additions & 2 deletions packages/qwik-router/src/runtime/src/route-loaders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import type {
LoaderConstructorQRL,
LoaderInternal,
LoaderOptions,
PathParams,
RequestEvent,
RequestEventLoader,
RouteNavigate,
Expand Down Expand Up @@ -118,12 +119,14 @@ const wrapWithAbort = <T>(promise: Promise<T>, signal: AbortSignal): Promise<T>
* 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<string, string | undefined>;
loaderParams: Record<string, PathParams | undefined>;
pagePathname?: string;
pageSearch?: string;
/** SPA navigation function. Client-only and intentionally omitted from SSR state. */
Expand Down Expand Up @@ -555,6 +558,7 @@ export function getRouteLoaderCtx(requestEv: RequestEventBase): RouteLoaderCtx {
if (!ctx) {
ctx = {
loaderPaths: {},
loaderParams: {},
};
requestEv.sharedMap.set(REQUEST_LOADER_PATHS_STORE, ctx);
}
Expand Down Expand Up @@ -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;
Expand All @@ -800,7 +807,7 @@ export const getLoaderRequestEvent = (
enumerable: true,
},
params: {
value: {},
value: scopedParams,
enumerable: true,
},
pathname: {
Expand Down
6 changes: 3 additions & 3 deletions packages/qwik-router/src/runtime/src/route-loaders.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand All @@ -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;
Expand All @@ -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;
Expand Down
42 changes: 36 additions & 6 deletions packages/qwik-router/src/runtime/src/routing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export const loadRoute = async (
routeBundleNames,
loaderHashes,
loaderPathsByHash,
loaderParamsByHash,
menuLoader,
errorLoader,
} = result;
Expand Down Expand Up @@ -72,6 +73,7 @@ export const loadRoute = async (
$errorLoader$: errorLoader,
$loaders$: loaderHashes,
$loaderPaths$: loaderPathsByHash,
$loaderParams$: loaderParamsByHash,
};
};

Expand Down Expand Up @@ -206,7 +208,9 @@ function collectNodeMeta(
menuLoaderRef: { v: MenuModuleLoader | undefined },
loaderHashes?: string[],
loaderPathsByHash?: Record<string, string>,
matchedPathname = '/'
matchedPathname = '/',
loaderParamsByHash?: Record<string, PathParams>,
matchedParams: PathParams = {}
) {
for (let j = 0; j < groups.length; j++) {
const g = groups[j];
Expand All @@ -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;
}
}
}
}
Expand All @@ -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;
}
}
}
}
Expand Down Expand Up @@ -477,6 +487,7 @@ function matchRouteTree(
routeBundleNames: string[] | undefined;
loaderHashes: string[] | undefined;
loaderPathsByHash: Record<string, string> | undefined;
loaderParamsByHash: Record<string, PathParams> | undefined;
menuLoader: MenuModuleLoader | undefined;
/** The nearest _E (error.tsx) boundary's chain to render on a thrown error (in its layouts). */
errorLoader: ModuleLoader[] | undefined;
Expand All @@ -487,6 +498,7 @@ function matchRouteTree(
const layouts: ModuleLoader[] = [];
const loaderHashes: string[] = [];
const loaderPathsByHash: Record<string, string> = {};
const loaderParamsByHash: Record<string, PathParams> = {};
const errorLoaderRef: BoundaryRef = { v: undefined, layouts: [] };
const notFoundLoaderRef: BoundaryRef = { v: undefined, layouts: [] };
const menuLoaderRef: { v: MenuModuleLoader | undefined } = { v: undefined };
Expand All @@ -503,7 +515,9 @@ function matchRouteTree(
notFoundLoaderRef,
menuLoaderRef,
loaderHashes,
loaderPathsByHash
loaderPathsByHash,
'/',
loaderParamsByHash
);
if (root._M) {
groupNodes.push({ node: root, depth: layouts.length });
Expand All @@ -528,6 +542,7 @@ function matchRouteTree(
params: PathParams;
layouts: ModuleLoader[];
loaderPathsByHash: Record<string, string>;
loaderParamsByHash: Record<string, PathParams>;
errorLoader: ContentModuleLoader | ModuleLoader[] | undefined;
errorLayouts: ModuleLoader[];
notFoundLoader: ContentModuleLoader | ModuleLoader[] | undefined;
Expand Down Expand Up @@ -561,6 +576,7 @@ function matchRouteTree(
params: { ...params },
layouts: [...layouts],
loaderPathsByHash: { ...loaderPathsByHash },
loaderParamsByHash: { ...loaderParamsByHash },
errorLoader: errorLoaderRef.v,
errorLayouts: errorLoaderRef.layouts,
notFoundLoader: notFoundLoaderRef.v,
Expand All @@ -586,7 +602,9 @@ function matchRouteTree(
menuLoaderRef,
loaderHashes,
loaderPathsByHash,
matchedPathname
matchedPathname,
loaderParamsByHash,
{ ...params }
);
if (node._M) {
groupNodes.push({ node, depth: layouts.length });
Expand All @@ -611,7 +629,9 @@ function matchRouteTree(
menuLoaderRef,
loaderHashes,
loaderPathsByHash,
pathname
pathname,
loaderParamsByHash,
{ ...params }
);
node = indexResult.target;
}
Expand All @@ -635,7 +655,9 @@ function matchRouteTree(
menuLoaderRef,
loaderHashes,
loaderPathsByHash,
pathname
pathname,
loaderParamsByHash,
{ ...params }
);
node = next;
}
Expand All @@ -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 };
Expand All @@ -668,7 +691,9 @@ function matchRouteTree(
fbMenuRef,
fbLoaderHashes,
fbLoaderPathsByHash,
pathname
pathname,
fbLoaderParamsByHash,
fbParams
);

const fbLoaders = resolveLoaders(root, fb.aNode, fbLayouts);
Expand All @@ -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),
};
Expand Down Expand Up @@ -737,6 +764,7 @@ function matchRouteTree(
routeBundleNames: undefined,
loaderHashes: undefined,
loaderPathsByHash: undefined,
loaderParamsByHash: undefined,
menuLoader: menuLoaderRef.v,
errorLoader: boundaryChain(errorLoaderRef),
};
Expand All @@ -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 };
}
}

Expand All @@ -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),
};
Expand Down
30 changes: 30 additions & 0 deletions packages/qwik-router/src/runtime/src/routing.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' };
Expand Down
2 changes: 2 additions & 0 deletions packages/qwik-router/src/runtime/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string>;
/** Runtime-only mapping of routeLoader$ hashes to params matched at their loader path */
$loaderParams$?: Record<string, PathParams>;
}

export interface EndpointResponse {
Expand Down