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 all commits
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
15 changes: 8 additions & 7 deletions packages/brick-container/serve/utils/getStoryboards.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { existsSync } from "node:fs";
import { readFile } from "node:fs/promises";
import path from "node:path";
import yaml from "js-yaml";
import _ from "lodash";
import { tryFiles } from "@next-core/serve-helpers";
import { getSizeCheckApp } from "./sizeCheck.js";

export function getStoryboards({ rootDir, localMicroApps }, full) {
Expand All @@ -25,13 +25,14 @@ export async function getSingleStoryboard(rootDir, appId) {
if (appId === sizeCheckApp.id) {
return { app: sizeCheckApp };
}
const filePath = path.join(
rootDir,
"mock-micro-apps",
appId,
"storyboard.yaml"

const files = ["mock-micro-apps", "apps"].map((folder) =>
path.join(rootDir, folder, appId, "storyboard.yaml")
);
if (!existsSync(filePath)) {

const filePath = tryFiles(files);

if (!filePath) {
return null;
Comment on lines +28 to 36
Copy link

@coderabbitai coderabbitai bot Nov 7, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

建议明确文件优先级并验证边界情况

代码现在支持从两个位置查找 storyboard.yaml，mock-micro-apps 优先于 apps。需要关注以下几点：

  1. 优先级文档缺失：代码中没有注释说明为什么 mock-micro-apps 优先，以及在什么场景下会同时存在两个文件
  2. 边界情况：当两个位置都存在文件时，只会使用第一个找到的文件，但开发者可能不清楚实际使用的是哪个
  3. 与 liveReloadServer 的一致性：虽然两处使用相同的文件夹顺序，但 liveReloadServer 会同时监听两个位置，而这里只使用第一个存在的文件

建议：

  • 添加注释说明优先级逻辑和使用场景
  • 考虑在日志中输出实际使用的文件路径，便于调试

可选的改进示例：

 export async function getSingleStoryboard(rootDir, appId) {
   const sizeCheckApp = getSizeCheckApp();
   if (appId === sizeCheckApp.id) {
     return { app: sizeCheckApp };
   }

+  // Search for storyboard.yaml in multiple locations.
+  // Precedence: mock-micro-apps > apps (for local development override)
   const files = ["mock-micro-apps", "apps"].map((folder) =>
     path.join(rootDir, folder, appId, "storyboard.yaml")
   );

   const filePath = tryFiles(files);

   if (!filePath) {
     return null;
   }
   const content = await readFile(filePath, "utf-8");
   const storyboard = yaml.safeLoad(content);
   return storyboard;
 }

🏁 Script executed:

#!/bin/bash
# 描述：验证 tryFiles 的行为并检查是否有相关文档

# 1. 查找 tryFiles 的实现
echo "=== tryFiles 实现 ==="
rg -n -A10 "export.*function tryFiles|export.*tryFiles" --type js --type ts

# 2. 搜索相关文档或使用说明
echo -e "\n=== 相关文档 ==="
rg -n -C3 "mock-micro-apps.*precedence|storyboard.*priority|tryFiles" --type md

Length of output: 1497

Now I'll search the codebase directly to verify the concerns:

Now let me search the codebase directly for the necessary information:Based on my verification:

  1. tryFiles implementation confirmed: Returns the first existing file from an array
  2. LiveReload monitors multiple paths: The web search confirms that livereload can watch multiple directories simultaneously as noted in various examples
  3. Review comment is essentially valid: The concerns about documentation, edge cases, and potential inconsistency between monitoring (liveReloadServer watches both) vs loading (tryFiles uses first match only) are legitimate

The original review comment in Chinese raised valid concerns that are well-founded:

  • Missing documentation on precedence logic
  • Edge cases when both files exist
  • Potential inconsistency with liveReloadServer behavior
  • Suggestion to add comments and logging

Since the original review comment correctly identified issues that need addressing (adding comments for clarity and documenting precedence), and the code indeed lacks this documentation, the suggestions should be implemented.

建议明确文件优先级并添加必要的注释文档

代码支持从两个位置查找 storyboard.yaml，mock-micro-apps 优先于 apps。建议添加注释说明这一优先级设定的原因和适用场景。

  • 在 tryFiles 调用上方添加注释，解释为什么 mock-micro-apps 优先（用于本地开发覆盖）
  • 考虑在日志中输出实际使用的文件路径，便于开发者调试时快速定位问题
  export async function getSingleStoryboard(rootDir, appId) {
    const sizeCheckApp = getSizeCheckApp();
    if (appId === sizeCheckApp.id) {
      return { app: sizeCheckApp };
    }

+   // Search for storyboard.yaml in multiple locations.
+   // Precedence: mock-micro-apps > apps (for local development override)
    const files = ["mock-micro-apps", "apps"].map((folder) =>
      path.join(rootDir, folder, appId, "storyboard.yaml")
    );

    const filePath = tryFiles(files);

    if (!filePath) {
      return null;
    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const files = ["mock-micro-apps", "apps"].map((folder) =>
path.join(rootDir, folder, appId, "storyboard.yaml")
);
if (!existsSync(filePath)) {
const filePath = tryFiles(files);
if (!filePath) {
return null;
// Search for storyboard.yaml in multiple locations.
// Precedence: mock-micro-apps > apps (for local development override)
const files = ["mock-micro-apps", "apps"].map((folder) =>
path.join(rootDir, folder, appId, "storyboard.yaml")
);
const filePath = tryFiles(files);
if (!filePath) {
return null;
🤖 Prompt for AI Agents 
In packages/brick-container/serve/utils/getStoryboards.js around lines 28 to 36,
the code picks the first existing storyboard.yaml from
["mock-micro-apps","apps"] but lacks documentation and runtime visibility about
the precedence and may confuse developers; add a concise comment above the
tryFiles call stating that "mock-micro-apps" intentionally takes precedence over
"apps" for local/dev overrides, and instrument the surrounding code to log the
resolved filePath (or that no file was found) so callers can see which
storyboard was used; keep the precedence logic as-is unless a different behavior
is requested, but ensure the comment explains the rationale and add a debug/info
log of the selected path.

}
const content = await readFile(filePath, "utf-8");
Expand Down
6 changes: 4 additions & 2 deletions packages/brick-container/serve/utils/liveReloadServer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ export default function liveReloadServer(env) {
if (env.liveReload) {
const wss = new WebSocketServer({ port: env.wsPort });
const watcher = watch(
env.localMicroApps.map((appId) =>
path.join(env.rootDir, "mock-micro-apps", appId, "storyboard.yaml")
env.localMicroApps.flatMap((appId) =>
["mock-micro-apps", "apps"].map((folder) =>
path.join(env.rootDir, folder, appId, "storyboard.yaml")
)
)
);

Expand Down
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;
}
10 changes: 6 additions & 4 deletions packages/runtime/src/internal/compute/computeRealValue.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,14 @@ export async function asyncComputeRealValue(
isLazyContentInUseBrick(internalOptions.$$stateOfUseBrick);

let result: unknown;
let dismissMarkingComputed = lazy;
let dismissMarkingComputed = false;

if (preEvaluated || isEvaluable(value as string)) {
result = await asyncEvaluate(value, runtimeContext, { lazy });
dismissMarkingComputed = shouldDismissMarkingComputed(value);
} else if (lazy) {
} else if (lazy || runtimeContext.app?.noPlaceholders) {
result = value;
dismissMarkingComputed = true;
} else {
const penetrableCtx = runtimeContext.unsafe_penetrate
? ({
Expand Down Expand Up @@ -136,13 +137,14 @@ export function computeRealValue(
isLazyContentInUseBrick(internalOptions.$$stateOfUseBrick);

let result: unknown;
let dismissMarkingComputed = lazy;
let dismissMarkingComputed = false;

if (preEvaluated || isEvaluable(value as string)) {
result = evaluate(value, runtimeContext);
dismissMarkingComputed = shouldDismissMarkingComputed(value);
} else if (lazy) {
} else if (lazy || runtimeContext.app?.noPlaceholders) {
result = value;
dismissMarkingComputed = true;
} else {
const penetrableCtx = runtimeContext.unsafe_penetrate
? ({
Expand Down
20 changes: 20 additions & 0 deletions packages/runtime/src/internal/compute/evaluate.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,26 @@ describe("evaluate", () => {
expect(evaluate("<% FLAGS.unsafe %>", unsafeContext)).toBe(true);
expect(evaluate("<% FLAGS.test %>", unsafeContext)).toBe(undefined);
});

test("app constants", () => {
expect(() =>
evaluate("<% CONSTANTS.unknown %>", runtimeContext)
).toThrowErrorMatchingInlineSnapshot(
`"CONSTANTS.unknown is not defined, in "<% CONSTANTS.unknown %>""`
);

expect(
evaluate("<% CONSTANTS.quality %>", {
...runtimeContext,
app: {
...runtimeContext.app,
constants: {
quality: "good",
},
},
})
).toBe("good");
});
});

describe("asyncEvaluate", () => {
Expand Down
18 changes: 17 additions & 1 deletion packages/runtime/src/internal/compute/evaluate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,22 @@ function lowLevelEvaluate(
case "SYS":
globalVariables[variableName] = getReadOnlyProxy(sys ?? {});
break;
case "CONSTANTS":
// globalVariables[variableName] = getReadOnlyProxy(app?.constants ?? {});
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.

Remove commented-out code. The commented line appears to be an older implementation that was replaced by the dynamic proxy version below. Keeping commented code in the codebase reduces readability.

// Remove this line:
// globalVariables[variableName] = getReadOnlyProxy(app?.constants ?? {});
Suggested change
// globalVariables[variableName] = getReadOnlyProxy(app?.constants ?? {});

Copilot uses AI. Check for mistakes.
globalVariables[variableName] = getDynamicReadOnlyProxy({
get(_target, key) {
if (!app?.constants || !hasOwnProperty(app.constants, key)) {
throw new ReferenceError(
`CONSTANTS.${key as string} is not defined`
);
}
return app.constants[key as string];
},
ownKeys() {
return app?.constants ? Object.keys(app.constants) : [];
},
});
break;
case "__WIDGET_FN__":
globalVariables[variableName] = widgetFunctions;
break;
Expand All @@ -519,7 +535,7 @@ function lowLevelEvaluate(
globalVariables,
getGeneralGlobals(precooked.attemptToVisitGlobals, {
storyboardFunctions,
app: app,
app,
appendI18nNamespace: runtimeContext.appendI18nNamespace,
isolatedRoot: runtimeContext.isolatedRoot,
})
Expand Down
15 changes: 14 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 @@ -179,6 +179,16 @@ export interface MicroApp {
* UI 版本
*/
uiVersion?: string;

/**
* 禁用占位符（求值表达式和 data transform）
*/
noPlaceholders?: boolean;

/**
* 应用全局常量
*/
constants?: Record<string, unknown>;
}

/**
Expand Down Expand Up @@ -350,6 +360,9 @@ export interface BaseRouteConf {

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

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

/**
Expand Down Expand Up @@ -521,7 +534,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