Make the agent-facing prop types airtight#30
Merged
Conversation
…`as const`
The canonical ColumnValues.border pins each per-side *Width to `${number}px`, so
an inline `border={{ borderBottomWidth: "1px" }}` type-checks but the recommended
DRY hairline pattern — a reusable `const HAIRLINE = { ... }` applied across many
columns — widens "1px" to `string` and fails strict tsc, even though the runtime
already accepts it. The types were stricter than the runtime, the opposite of the
DX layer's intent.
Add a BorderInput type (derived from the canonical border shape so it tracks the
schema instead of duplicating it) that relaxes the per-side *Width fields to
SizeInput, and Omit+redeclare `border` on ColumnProps — mirroring the existing
FontFamilyInput / SizeInput widenings. Type-only change; build green, 329 tests
pass. Button/Table/Divider carry the same canonical border and can reuse
BorderInput in a follow-up.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The build only type-checks the index import graph, so the agent-friendly prop types had no CI guard — a relaxed input type could be reverted (like the Column `border` regression) and nothing would catch it; vitest doesn't type-check. Add src/dx-types.test-d.tsx: compile-time assertions that the natural authoring forms type-check (factored-out border hairline incl. numeric width, numeric/ string fontSize+fontWeight, string/object fontFamily, numeric lineHeight, full- width button, percent image width) and that garbage is rejected (@ts-expect-error on a string border, a bogus fontWeight). It imports only TYPES, so it stays out of the render graph and checks in isolation — no storybook/exporters noise. Wire it up with a scoped tsconfig.typecheck.json, a `typecheck` script, and a "Type contract" CI step. Verified the gate goes red when BorderInput is reverted to the strict canonical type. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A type stress test of every component showed the Column `border` fix was one
instance of a broader hazard: the canonical @unlayer/types pins every scalar
dimension field to a `${number}px`-style template, so natural forms an agent
writes — a bare number (`borderRadius={8}`, `padding={14}`), a factored-out
border object, a computed/widened string — fail strict tsc even though the
runtime already normalizes them to px (verified: no unitless output, no
[object Object], no NaN). The types were stricter than the runtime.
Relax to SizeInput / BorderInput, mirroring the existing FontFamilyInput /
SizeInput widenings, at BOTH layers:
- component-local *SemanticProps (Button, Menu, Table, Divider) — these are
what the factory types the components with (what JSX checks);
- the exported *Props aliases in types.ts (Button/Menu/Table/Divider) and the
container props (Column/Body borderRadius).
Fields: borderRadius (Button/Column/Body), padding (Button/Menu/Table),
border object (Button/Table/Divider). Divider previously passed the raw strict
SemanticProps to the factory; it now has a relaxed DividerSemanticProps. Body
and Divider gained the `as SemanticProps<X>` cast at the mapSemanticProps call
that Column/Row already use. Type-only; build green, 329 tests pass, render
output unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tions Guard the broader relaxation against the ACTUAL component prop types (imported from the component files, not just the exported aliases): borderRadius as a number on Column/Button, item padding as a number on Button/Menu/Table, and a factored-out border object on Button/Table/Divider. Runs in the existing `typecheck` CI gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The item prop types (ButtonProps, HeadingProps, …) were defined twice — a parallel set in types.ts (the exported ones) and the real ones next to each component (what the components are actually typed with). The two could drift, so the type you imported didn't always match what the component accepted. Drop the types.ts duplicates and export each component's own prop type from its file, matching how RowProps / EmailProps already work. types.ts is now just the shared value-type re-exports plus the agent-friendly input building blocks (SizeInput, BorderInput, TextStyleProps, …) that components import, and the unused ItemProps helper is gone. No behavior change; build green, 329 tests pass, and the exported type now equals the component's accepted props. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…contract extract-head.ts imported `type ComponentHead` from @unlayer/exporters, but that package only exports `heads` — an untyped Record<string, any> registry — and neither @unlayer/exporters nor @unlayer/types defines a head type. The phantom import was a latent type error the build happened to tolerate. Replace it with a local ComponentHead type describing the css/js/tags builders this file calls. The index import graph is now type-clean, so add dx-contract.test.tsx to the typecheck gate — its @ts-expect-error garbage-rejection assertions are now enforced in CI alongside dx-types.test-d.tsx. Type-only; build green, 329 tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the React elements package’s public prop type surface to better match the runtime’s “natural authoring forms” (numbers → px, factored-out objects, etc.), centralizes component prop type sources, and adds a CI type-contract gate to prevent regressions in agent-friendly typings.
Changes:
- Introduces shared “input” building blocks (
SizeInput,BorderInput, etc.) and relaxes box-model-related prop types across components. - Makes each component file the source of truth for its exported
*Propstype and updates the package’s type exports accordingly. - Adds a dedicated
tsctype-contract step (with new contract files + CI wiring) to assert good inputs compile and bad inputs fail.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react/tsconfig.typecheck.json | Adds a dedicated tsc --noEmit project for DX type-contract compilation. |
| packages/react/src/utils/extract-head.ts | Removes a non-existent upstream type import and replaces it with a local head-shape type. |
| packages/react/src/types.ts | Reframes this module as shared “agent-friendly input” building blocks + value-type re-exports. |
| packages/react/src/index.ts | Switches component prop-type exports to come directly from each component module. |
| packages/react/src/dx-types.test-d.tsx | Adds compile-time-only assertions for widened/relaxed prop inputs and rejected garbage. |
| packages/react/src/components/Table.tsx | Widens padding/border inputs and introduces TableSemanticProps for the mapper. |
| packages/react/src/components/Menu.tsx | Widens padding input and refactors MenuProps to be based on MenuSemanticProps. |
| packages/react/src/components/Divider.tsx | Widens border input and updates the item component generic prop type. |
| packages/react/src/components/Column.tsx | Widens padding/borderRadius/border inputs for Column props. |
| packages/react/src/components/Button.tsx | Widens padding/borderRadius/border inputs for Button props. |
| packages/react/src/components/Body.tsx | Widens borderRadius input and updates semantic-props mapping call typing. |
| packages/react/package.json | Adds a typecheck script to run the DX type-contract compilation. |
| .github/workflows/test.yml | Adds a CI job step to run the new type-contract gate. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { mapSemanticProps, type SemanticProps } from "../utils/semantic-props"; | ||
|
|
||
| type MenuSemanticProps = SemanticProps<MenuValues> & { | ||
| type MenuSemanticProps = Omit<SemanticProps<MenuValues>, "padding"> & { |
Comment on lines
+6
to
25
| type TableSemanticProps = Omit<SemanticProps<TableValues>, "padding" | "border"> & { | ||
| /** Column headers as string[] */ | ||
| headers?: string[]; | ||
| /** Row data as 2D string array */ | ||
| data?: string[][]; | ||
| /** Inner padding — a number (→ px) or CSS string. */ | ||
| padding?: SizeInput; | ||
| /** Per-side border object (width fields accept a number/px string). */ | ||
| border?: BorderInput; | ||
| }; | ||
|
|
||
| export interface TableProps extends Omit<ItemComponentProps<SemanticProps<TableValues>>, "headers" | "data"> { | ||
| export interface TableProps | ||
| extends Omit<ItemComponentProps<Omit<SemanticProps<TableValues>, "padding" | "border">>, "headers" | "data"> { | ||
| headers?: string[]; | ||
| data?: string[][]; | ||
| /** Inner padding — a number (→ px) or CSS string. */ | ||
| padding?: SizeInput; | ||
| /** Per-side border object (width fields accept a number/px string). */ | ||
| border?: BorderInput; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Agents (and people) writing templates in code kept hitting cases where natural prop forms type-checked wrong or rendered broken, even though the runtime handled them fine — the canonical value types are stricter than the runtime. This was the root of the rough code-generation experience.
What
A type stress test of every component showed one broad class of issue: every scalar dimension field is pinned to a
${number}px-style template, so natural forms fail stricttscwhile the runtime already normalizes them.borderRadius, itempadding, andborderobjects now accept a number (→ px) or any CSS string / factored-out object, viaSizeInput/BorderInput(mirrors the existingfontSize/fontFamilywidenings). Type-only — the runtime already normalized these.XPropsintypes.ts; each component’s prop type is exported from its own file.import type { ButtonProps }now is what<Button>accepts.types.tsis now just value re-exports + the shared input building blocks.extract-head.ts— replaced a phantomComponentHeadimport (no such type exists upstream) with a local type for the head builders it calls. The real source is nowtsc-clean.pnpm typecheckoverdx-types.test-d.tsx+dx-contract.test.tsxasserts natural forms compile and garbage is rejected.tsup/vitestdon’t type-check these, so this is the only guard against a relaxed type being reverted.Verification
build✅ ·typecheck✅ (both contract files) · 329/329 tests ✅[object Object], no unitless output); structural mistakes fail with clear messages.Release
Backward-compatible — types are only widened, no runtime change. Ships fine as a patch.