Skip to content

refactor(core): split layout prop validation by concern#279

Merged
RtlZeroMemory merged 1 commit intomainfrom
codex/split-layout-validate-props
Mar 17, 2026
Merged

refactor(core): split layout prop validation by concern#279
RtlZeroMemory merged 1 commit intomainfrom
codex/split-layout-validate-props

Conversation

@RtlZeroMemory
Copy link
Copy Markdown
Owner

@RtlZeroMemory RtlZeroMemory commented Mar 17, 2026

Summary

  • Split packages/core/src/layout/validateProps.ts into internal validation modules.
  • Kept the public API unchanged.
  • No intended behavior change.

Why

  • Reduce layout validation monolith size before deeper layout engine refactors.

Validation

  • npm run lint
  • npm run typecheck
  • npm run build
  • node scripts/run-tests.mjs --filter "packages/core/dist/layout/tests/"
  • node scripts/run-tests.mjs --filter "packages/core/dist/widgets/tests/vnode.prop-validation"
  • node scripts/run-tests.mjs

Summary by CodeRabbit

  • Chores
    • Refactored layout component validation infrastructure for improved code organization and maintainability. Enhanced internal error handling and validation patterns across layout primitives (stacks, boxes, buttons, inputs, sliders, and text components) to ensure more consistent and robust property validation.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 17, 2026

📝 Walkthrough

Walkthrough

Introduces a modular layout validation architecture by decomposing layout property validation across five specialized modules (shared, primitives, spacing, layoutConstraints, interactive), then re-exports from a central barrel file. Moves approximately 1,188 lines of inline validation code to dedicated, cohesive validator modules without changing runtime behavior.

Changes

Cohort / File(s) Summary
Error Handling & Type System
packages/core/src/layout/validate/shared.ts
Adds LayoutResult discriminated union, InvalidPropsFatal error type, and helper functions for consistent error propagation across all validators.
Primitive Value Validators
packages/core/src/layout/validate/primitives.ts
Implements type coercion and validation for numbers, strings, booleans, spacing, size constraints, flex, alignment, position, and grid placement with responsive value support and 32-bit integer bounds enforcement.
Spacing & Constraint Validation
packages/core/src/layout/validate/spacing.ts, packages/core/src/layout/validate/layoutConstraints.ts
Adds validated spacing props for padding/margin (with sign-aware constraints) and layout constraint aggregation for size, flex, alignment, positioning, and grid properties.
Interactive Component Validators
packages/core/src/layout/validate/interactive.ts
Implements validators for ten component types (Stack, Box, Spacer, Button, Input, Select, Slider, Checkbox, RadioGroup, Text), each returning typed validated props with warning emission support for edge cases.
Public API Re-export
packages/core/src/layout/validateProps.ts
Refactors from monolithic (~1,188 lines) to modular re-export architecture; consolidates public types and validator functions from five submodules into unified barrel exports.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • PR #193: Directly overlaps in validation surface and LayoutResult error handling pattern used by validateSelectProps and shared validation utilities.
  • PR #234: Complements constraint system by pairing the constraint expression parser and resolver with the new ConstraintExpr-aware validators (validateLayoutConstraints, requireSizeConstraint, etc.).
  • PR #186: Related through overlapping layout constraint types and property validators (flexShrink, alignSelf, position, grid placement, wrap, gap) that extend the layout engine surface.

Poem

🐰 Hop along, dear validation spread!
Once tangled monoliths, now modular instead.
From shared to spacing, each module sings clear—
A symphony of validators, organized and sincere. 🎻✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: refactoring the layout prop validation module by splitting it into smaller, concern-based modules.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/split-layout-validate-props
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

Migrating from UI to YAML configuration.

Use the @coderabbitai configuration command in a PR comment to get a dump of all your UI settings in YAML format. You can then edit this YAML file and upload it to the root of your repository to configure CodeRabbit programmatically.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
packages/core/src/layout/validateProps.ts (1)

19-47: Please attach the mandatory layout PTY/frame-audit evidence.

This rewires the core layout validation path, and the PR notes lint/typecheck/build/tests but not the live PTY validation the repo asks for on layout changes. Please include the REZI_FRAME_AUDIT / frame-audit-report.mjs evidence before merge.

Based on learnings: runtime/router/reconcile/layout/drawlist changes (high risk) need full test suite + integration coverage + PTY evidence; for rendering/layout/theme regressions, include mandatory live PTY validation: run app in PTY with deterministic viewport, exercise routes, capture REZI_FRAME_AUDIT logs, analyze with frame-audit-report.mjs, include concrete evidence in report.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/layout/validateProps.ts` around lines 19 - 47, This change
rewires core layout validation (exports in validateProps.ts such as
validateBoxProps, validateStackProps, validateTextProps, etc.), so run the
mandated live PTY frame-audit: start the app in a deterministic PTY viewport
with REZI_FRAME_AUDIT enabled, exercise relevant routes/components that touch
the changed validators (box/stack/text/slider/etc.), capture the
REZI_FRAME_AUDIT logs, run frame-audit-report.mjs to analyze them, and attach
the generated frame-audit-report.mjs output (and raw REZI_FRAME_AUDIT logs) to
the PR before merge.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/core/src/layout/validate/interactive.ts`:
- Around line 172-176: The validator functions currently cast arbitrary inputs
to prop bags (e.g., const p = (props ?? {}) as StackPropBag) which lets
non-object inputs like numbers/strings/arrays silently pass; change each
validator (validateStackProps, validateBoxProps, validateSpacerProps,
validateButtonProps, validateInputProps, validateSelectProps,
validateSliderProps, validateCheckboxProps, validateRadioGroupProps,
validateTextProps) to first check that props is a plain object (typeof props ===
"object" && props !== null && !Array.isArray(props)); if the check fails return
a failing LayoutResult, otherwise set p = props as <appropriate> and proceed to
apply defaults and validations—replace the existing (props ?? {}) as ... cast in
the listed functions with this guard so bad callers fail consistently.

In `@packages/core/src/layout/validate/shared.ts`:
- Around line 22-31: invalidProp currently uses String(received) which can run
user-defined coercion and throw; change invalidProp (and/or add a small helper
used by it) to safely stringify received by wrapping String(received) in a
try/catch (or perform a non-coercing check for primitives and fallback) and use
a safe fallback like "<unserializable>" when coercion throws, then pass that
safe string into invalid along with describeReceivedType to ensure validation
returns a LayoutResult instead of throwing; update references to invalidProp,
describeReceivedType, and invalid accordingly.

---

Nitpick comments:
In `@packages/core/src/layout/validateProps.ts`:
- Around line 19-47: This change rewires core layout validation (exports in
validateProps.ts such as validateBoxProps, validateStackProps,
validateTextProps, etc.), so run the mandated live PTY frame-audit: start the
app in a deterministic PTY viewport with REZI_FRAME_AUDIT enabled, exercise
relevant routes/components that touch the changed validators
(box/stack/text/slider/etc.), capture the REZI_FRAME_AUDIT logs, run
frame-audit-report.mjs to analyze them, and attach the generated
frame-audit-report.mjs output (and raw REZI_FRAME_AUDIT logs) to the PR before
merge.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1971379a-d804-460a-87e7-8edc3db4c04e

📥 Commits

Reviewing files that changed from the base of the PR and between a76dd79 and 43ca7cc.

📒 Files selected for processing (6)
  • packages/core/src/layout/validate/interactive.ts
  • packages/core/src/layout/validate/layoutConstraints.ts
  • packages/core/src/layout/validate/primitives.ts
  • packages/core/src/layout/validate/shared.ts
  • packages/core/src/layout/validate/spacing.ts
  • packages/core/src/layout/validateProps.ts

Copy link
Copy Markdown
Owner Author

@RtlZeroMemory RtlZeroMemory left a comment

Choose a reason for hiding this comment

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

Reviewed the automated comments against the scope of this PR.

@RtlZeroMemory RtlZeroMemory merged commit 407993c into main Mar 17, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant