Skip to content

fix(): handle sibling routes matching order #4825

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
weareoutman merged 1 commit into from
Nov 18, 2025
Merged

fix(): handle sibling routes matching order #4825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
57 changes: 41 additions & 16 deletions packages/runtime/src/internal/Renderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import type {
RenderReturnNode,
RuntimeBrickConfWithSymbols,
RuntimeContext,
RouteNode,
} from "./interfaces.js";
import {
getTagNameOfCustomTemplate,
Expand Down Expand Up @@ -99,7 +100,7 @@ export async function renderRoutes(
routes: RouteConf[],
_runtimeContext: RuntimeContext,
rendererContext: RendererContext,
parentRoutes: RouteConf[],
parentRoutes: RouteNode[],
menuRequestReturnNode: MenuRequestNode,
slotId?: string,
isIncremental?: boolean,
Expand Down Expand Up @@ -131,7 +132,7 @@ export async function renderRoutes(
if (isIncremental) {
runtimeContext.ctxStore.disposeDataInRoutes(routes);
}
const routePath = parentRoutes.concat(route);
const routePath = parentRoutes.concat({ route, routes });
runtimeContext.ctxStore.define(
route.context,
runtimeContext,
Expand Down Expand Up @@ -246,7 +247,7 @@ export async function renderBricks(
bricks: BrickConf[],
runtimeContext: RuntimeContext,
rendererContext: RendererContext,
parentRoutes: RouteConf[],
parentRoutes: RouteNode[],
menuRequestReturnNode: MenuRequestNode,
slotId?: string,
tplStack?: Map<string, number>,
Expand Down Expand Up @@ -283,7 +284,7 @@ export async function renderBrick(
brickConf: RuntimeBrickConfWithSymbols,
_runtimeContext: RuntimeContext,
rendererContext: RendererContext,
parentRoutes: RouteConf[],
parentRoutes: RouteNode[],
menuRequestReturnNode: MenuRequestNode,
slotId?: string,
tplStack = new Map<string, number>(),
Expand Down Expand Up @@ -320,7 +321,7 @@ async function legacyRenderBrick(
brickConf: RuntimeBrickConfWithSymbols,
_runtimeContext: RuntimeContext,
rendererContext: RendererContext,
parentRoutes: RouteConf[],
parentRoutes: RouteNode[],
menuRequestReturnNode: MenuRequestNode,
slotId: string | undefined,
tplStack: Map<string, number>,
Expand Down Expand Up @@ -912,30 +913,34 @@ async function legacyRenderBrick(
};
lastOutput.node = abstractNode;

const parentRoute = parentRoutes[parentRoutes.length - 1] as
| RouteConfOfBricks
| undefined;
if (parentRoute?.incrementalSubRoutes) {
const { route: parentRoute } = parentRoutes[parentRoutes.length - 1];
if ((parentRoute as RouteConfOfBricks)?.incrementalSubRoutes) {
routeSlotFromIndexToSlotId.set(index, childSlotId);
rendererContext.performIncrementalRender(
slotConf,
parentRoutes,
parentRoutes.map(({ route }) => route),
async (location, prevLocation) => {
const { homepage } = childRuntimeContext.app;
const { pathname } = location;
// Ignore if any one of homepage and parent routes not matched.
if (
!matchHomepage(homepage, pathname) ||
!parentRoutes.every((route) => {
let prevMatch: MatchResult | null;
let newMatch: MatchResult | null;
!parentRoutes.every(({ route, routes }) => {
let prevMatch: MatchResult | null = null;
let newMatch: MatchResult | null = null;
Comment on lines +929 to +930
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The explicit initialization to null is unnecessary here since the variables are immediately assigned in the return statement on lines 933 and 939. The original code without initialization was sufficient. However, if the intent is to make the null-ability more explicit for readability, this is acceptable but adds no functional value.

Suggested change
let prevMatch: MatchResult | null = null;
let newMatch: MatchResult | null = null;
let prevMatch: MatchResult | null;
let newMatch: MatchResult | null;

Copilot uses AI. Check for mistakes.
return (
(prevMatch = matchRoute(
(prevMatch = matchTargetRoute(
route,
routes,
homepage,
prevLocation.pathname
)) &&
(newMatch = matchRoute(route, homepage, pathname)) &&
(newMatch = matchTargetRoute(
route,
routes,
homepage,
pathname
)) &&
(route !== parentRoute ||
isRouteParamsEqual(prevMatch.params, newMatch.params))
);
Expand Down Expand Up @@ -1124,7 +1129,7 @@ async function renderForEach(
bricks: BrickConf[],
runtimeContext: RuntimeContext,
rendererContext: RendererContext,
parentRoutes: RouteConf[],
parentRoutes: RouteNode[],
menuRequestReturnNode: MenuRequestNode,
slotId: string | undefined,
tplStack: Map<string, number>,
Expand Down Expand Up @@ -1369,3 +1374,23 @@ function isRouteParamsEqual(
function isRouteConf(child: BrickConf | RouteConf): child is RouteConf {
return !!(child as RouteConf).path && !(child as BrickConf).brick;
}

/**
* Find the first matched route among siblings.
*
* Return the match result only if it matches the target route.
*/
function matchTargetRoute(
route: RouteConf,
routes: RouteConf[],
homepage: string,
pathname: string
) {
for (const sibling of routes) {
const match = matchRoute(sibling, homepage, pathname);
if (match) {
return sibling === route ? match : null;
}
}
return null;
}
9 changes: 5 additions & 4 deletions packages/runtime/src/internal/data/DataStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { ResolveOptions, resolveData } from "./resolveData.js";
import { resolveDataStore } from "./resolveDataStore.js";
import type {
AsyncPropertyEntry,
RouteNode,
RuntimeBrick,
RuntimeContext,
} from "../interfaces.js";
Expand Down Expand Up @@ -330,7 +331,7 @@ export class DataStore<T extends DataStoreType = "CTX"> {
dataConfs: ContextConf[] | undefined,
runtimeContext: RuntimeContext,
asyncHostPropertyEntries?: AsyncPropertyEntry[],
routePath?: RouteConf[]
routePath?: RouteNode[]
): void {
if (Array.isArray(dataConfs) && dataConfs.length > 0) {
const pending = resolveDataStore(
Expand All @@ -346,7 +347,7 @@ export class DataStore<T extends DataStoreType = "CTX"> {
isStrictMode(runtimeContext)
);
if (Array.isArray(routePath)) {
for (const route of routePath) {
for (const { route } of routePath) {
const stack = this.routeStackMap.get(route);
if (stack) {
stack.add(pending);
Expand Down Expand Up @@ -414,7 +415,7 @@ export class DataStore<T extends DataStoreType = "CTX"> {
dataConf: ContextConf,
runtimeContext: RuntimeContext,
asyncHostPropertyEntries?: AsyncPropertyEntry[],
routePath?: RouteConf[]
routePath?: RouteNode[]
): Promise<boolean> {
if (!(await asyncCheckIf(dataConf, runtimeContext))) {
return false;
Expand Down Expand Up @@ -565,7 +566,7 @@ export class DataStore<T extends DataStoreType = "CTX"> {
this.data.set(dataConf.name, newData);

if (Array.isArray(routePath)) {
for (const route of routePath) {
for (const { route } of routePath) {
const names = this.routeMap.get(route);
if (names) {
names.add(dataConf.name);
Expand Down
6 changes: 6 additions & 0 deletions packages/runtime/src/internal/interfaces.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,3 +203,9 @@ export interface RuntimeDataVale
export interface RuntimeDataValueOption {
routeId?: string;
}

export interface RouteNode {
route: RouteConf;
// All ordered sibling routes under the same parent including the route itself
routes: RouteConf[];
}
Loading