Skip to content

fix(): support route children #4816

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 4 commits into from
Nov 7, 2025
Merged

fix(): support route children #4816

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
04982bb
fix(): support route children
weareoutman Nov 3, 2025
d1f137f
fix(): disable placeholders at app level
weareoutman Nov 3, 2025
1dea7c7
fix(): support app constants
weareoutman Nov 6, 2025
bbf240b
fix(): support apps dir
weareoutman Nov 7, 2025
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
54 changes: 48 additions & 6 deletions packages/runtime/src/internal/Renderer.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,12 +474,7 @@ describe("renderRoutes", () => {
bricks: [
{
brick: "div",
slots: {
"": {
type: "routes",
routes: [subRoute],
},
},
children: [subRoute],
},
],
};
Expand Down Expand Up @@ -533,6 +528,53 @@ describe("renderRoutes", () => {
expect(ctxStore.getValue("objectId")).toBe("HOST");
});

test("Slot conflict", async () => {
const renderRoot = {
tag: RenderTag.ROOT,
} as RenderRoot;
const ctxStore = new DataStore("CTX");
const runtimeContext = {
ctxStore,
location: {
pathname: "/home/HOST/list",
},
app: {
homepage: "/home",
noAuthGuard: true,
},
pendingPermissionsPreCheck: [] as undefined[],
flags: {},
} as RuntimeContext;
const rendererContext = new RendererContext("page");
const brick = { brick: "div" };
const subRoute: RouteConf = {
path: "${APP.homepage}/:objectId/list",
exact: true,
bricks: [brick],
};
const route: RouteConf = {
path: "${APP.homepage}/:objectId",
exact: false,
context: [{ name: "objectId", value: "<% PATH.objectId %>" }],
bricks: [
{
brick: "div",
children: [
subRoute,
{
brick: "hr",
},
],
},
],
};
expect(() =>
renderRoutes(renderRoot, [route], runtimeContext, rendererContext, [], {})
).rejects.toMatchInlineSnapshot(
`[Error: Slot "" conflict between bricks and routes.]`
);
});

test("missed", async () => {
const runtimeContext = {
location: {
Expand Down
22 changes: 17 additions & 5 deletions packages/runtime/src/internal/Renderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1296,7 +1296,7 @@ function getEmptyRenderOutput(): RenderOutput {
}

export function childrenToSlots(
children: BrickConf[] | undefined,
children: (BrickConf | RouteConf)[] | undefined,
originalSlots: SlotsConf | undefined
) {
let newSlots = originalSlots;
Expand All @@ -1317,13 +1317,21 @@ export function childrenToSlots(
newSlots = {};
for (const { slot: sl, ...child } of children) {
const slot = sl ?? "";
if (!hasOwnProperty(newSlots, slot)) {
const type = isRouteConf(child) ? "routes" : "bricks";
if (hasOwnProperty(newSlots, slot)) {
const slotConf = newSlots[slot];
if (slotConf.type !== type) {
throw new Error(`Slot "${slot}" conflict between bricks and routes.`);
}
(slotConf as SlotConfOfBricks)[type as "bricks"].push(
child as BrickConf
);
Comment on lines +1326 to +1328
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

Type casting issue: When type is "routes", the code attempts to push child as BrickConf into a routes array, which should contain RouteConf elements instead. This will cause type inconsistency.

The line should be:

(slotConf as SlotConfOfBricks | SlotConfOfRoutes)[type].push(child as any);

Or handle routes separately:

if (type === "bricks") {
  (slotConf as SlotConfOfBricks).bricks.push(child as BrickConf);
} else {
  (slotConf as SlotConfOfRoutes).routes.push(child as RouteConf);
}

Copilot uses AI. Check for mistakes.
} else {
newSlots[slot] = {
type: "bricks",
bricks: [],
type: type as "bricks",
[type as "bricks"]: [child as BrickConf],
Comment on lines 1330 to +1332
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

Type casting issue: When creating a new slot with type as "routes", the code casts type to "bricks" and child to BrickConf, which is incorrect. This should handle both cases properly:

newSlots[slot] = {
  type,
  [type]: [child],
} as SlotConf;

Or handle them separately:

if (type === "bricks") {
  newSlots[slot] = {
    type: "bricks",
    bricks: [child as BrickConf],
  };
} else {
  newSlots[slot] = {
    type: "routes",
    routes: [child as RouteConf],
  };
}

Copilot uses AI. Check for mistakes.
};
}
(newSlots[slot] as SlotConfOfBricks).bricks.push(child);
}
}
return newSlots;
Expand Down Expand Up @@ -1357,3 +1365,7 @@ function isRouteParamsEqual(
const d = omitBy(b, omitNumericKeys);
return isEqual(c, d);
}

function isRouteConf(child: BrickConf | RouteConf): child is RouteConf {
return !!(child as RouteConf).path && !(child as BrickConf).brick;
}
5 changes: 4 additions & 1 deletion packages/types/src/manifest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,9 @@ export interface BaseRouteConf {

/** 构件编排 ID */
iid?: string;

/** 插槽，用于构件下的路由 */
slot?: string;
}

/**
Expand Down Expand Up @@ -521,7 +524,7 @@ export interface BrickConf {
slots?: SlotsConf;

/** 子构件列表，优先级低于 `slots` */
children?: BrickConf[];
children?: (BrickConf | RouteConf)[];

/** 当使用 children 而不是 slots 定义子构件时，子构件需要设置所属 slot */
slot?: string;
Expand Down
2 changes: 1 addition & 1 deletion packages/utils/src/storyboard/replaceUseChildren.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ function replaceInBrick(brick: BrickConf) {
const useChildrenMap = new Map<string, BrickConf[]>();
if (Array.isArray(brick.children) && !slots) {
const removeBricks: BrickConf[] = [];
for (const child of brick.children) {
for (const child of brick.children as BrickConf[]) {
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

Type safety concern: The type assertion as BrickConf[] assumes that brick.children only contains BrickConf items, but the type now allows (BrickConf | RouteConf)[]. If a RouteConf is present, it will cause issues at line 41 where replaceInBrick(child) is called, which expects a BrickConf.

Consider adding a runtime check or filter:

for (const child of brick.children.filter((c): c is BrickConf => !!(c as BrickConf).brick)) {

Or add a type guard check:

for (const child of brick.children as BrickConf[]) {
  if (!(child as BrickConf).brick) continue; // Skip non-bricks
Suggested change
for (const child of brick.children as BrickConf[]) {
for (const child of brick.children.filter((c): c is BrickConf => !!(c as BrickConf).brick)) {

Copilot uses AI. Check for mistakes.
const slot = child.slot ?? "";
if (USE_CHILDREN_SLOT_REGEXP.test(slot)) {
delete child.slot;
Expand Down
Loading