refactor(core): split stack layout engine by concern#280
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis PR refactors the stack layout implementation from a monolithic 1987-line file into modularized components: separate modules for axis handling, constraint planning, layout rendering, measurement, wrapping, and shared utilities. The original Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/src/layout/kinds/stack.ts (1)
11-16: Consider importing the existinginvalid()helper instead of duplicating it.There's an identical
invalid()function inpackages/core/src/layout/validate/shared.ts(lines 11-13). To reduce code duplication and maintain consistency, consider importing it from there.♻️ Proposed refactor
import type { VNode } from "../../widgets/types.js"; import type { LayoutTree } from "../engine/types.js"; import type { Axis, Size } from "../types.js"; import type { LayoutResult } from "../validateProps.js"; +import { invalid } from "../validate/shared.js"; import { getAxisConfig } from "./stackParts/axis.js"; import { layoutStack } from "./stackParts/layout.js"; import { measureStack } from "./stackParts/measure.js"; import type { LayoutNodeFn, MeasureNodeFn } from "./stackParts/shared.js"; import { isStackVNode } from "./stackParts/shared.js"; -function invalid(detail: string): LayoutResult<never> { - return { - ok: false as const, - fatal: { code: "ZRUI_INVALID_PROPS" as const, detail }, - }; -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/layout/kinds/stack.ts` around lines 11 - 16, Remove the duplicated invalid() helper in stack.ts and import the existing invalid helper from the validate/shared module instead: delete the local function `invalid(detail: string): LayoutResult<never>` and add an import for the shared `invalid` so calls in this file use the single shared implementation; ensure the imported symbol matches the exported name in `packages/core/src/layout/validate/shared.ts` and that `LayoutResult` typings remain compatible, then run tests/typecheck to confirm no type regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/src/layout/kinds/stack.ts`:
- Around line 11-16: Remove the duplicated invalid() helper in stack.ts and
import the existing invalid helper from the validate/shared module instead:
delete the local function `invalid(detail: string): LayoutResult<never>` and add
an import for the shared `invalid` so calls in this file use the single shared
implementation; ensure the imported symbol matches the exported name in
`packages/core/src/layout/validate/shared.ts` and that `LayoutResult` typings
remain compatible, then run tests/typecheck to confirm no type regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2dc7df38-0978-4ec1-a5ed-d913d7101fb7
📒 Files selected for processing (7)
packages/core/src/layout/kinds/stack.tspackages/core/src/layout/kinds/stackParts/axis.tspackages/core/src/layout/kinds/stackParts/constraintPlan.tspackages/core/src/layout/kinds/stackParts/layout.tspackages/core/src/layout/kinds/stackParts/measure.tspackages/core/src/layout/kinds/stackParts/shared.tspackages/core/src/layout/kinds/stackParts/wrap.ts
Summary
packages/core/src/layout/kinds/stack.tsinto internal modules.Why
Validation
npm installnpm run lintnpm run typechecknpm run buildnode scripts/run-tests.mjs --scope packages --filter "packages/core/dist/layout/__tests__/"node scripts/run-tests.mjsnpm run build:nativestty rows 68 cols 300thenenv -u NO_COLOR REZI_STARSHIP_EXECUTION_MODE=worker REZI_STARSHIP_DEBUG=1 REZI_STARSHIP_DEBUG_LOG=/tmp/starship-stack.log REZI_FRAME_AUDIT=1 REZI_FRAME_AUDIT_LOG=/tmp/rezi-frame-audit-stack.ndjson npx tsx packages/create-rezi/templates/starship/src/main.tsnode scripts/frame-audit-report.mjs /tmp/rezi-frame-audit-stack.ndjson --latest-pidPTY/frame-audit evidence
300x68after startup negotiation; starship log recorded the route flowbridge -> engineering -> crew -> cargo, pluscycle-theme,cargo-sort-quantity, andquitcommands.records=17221,backend_submitted=818,worker_payload=818,worker_accepted=818,worker_completed=818.hash_mismatch_backend_vs_worker=0andmissing_worker_payload=0.bridge submitted=306 completed=306,engineering submitted=443 completed=443,crew submitted=41 completed=41,cargo submitted=28 completed=28.native_summary_records=818,native_header_records=1636.Summary by CodeRabbit