Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
43 changes: 43 additions & 0 deletions .changeset/6541-sonner-button-variant-enum.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
---
'@object-ui/types': minor
---

**Accept-set NARROWING on a published surface.** `SonnerSchema.buttonVariant` in the zod
mirror (`@object-ui/types/zod`) was `z.string()`; it is now
`z.enum(['default', 'secondary', 'destructive', 'outline', 'ghost', 'link'])`. Values
outside those six — `'primary'`, `'danger'`, `'Default'`, `''` — used to validate and now
fail. Stated as a narrowing rather than as a fix, because it removes values the published
mirror accepted (objectui#6541).

**What was wrong.** The same key on the same component shipped as two disagreeing published
faces: an open string to anyone validating (`@object-ui/types/zod`), and a closed
six-member union to anyone type-checking (`@object-ui/types`, `SonnerSchema.buttonVariant`
in `feedback.ts`). The TS face was already correct — only the mirror is changed here, so
this is the mirror being made to agree with a declaration that sat beside it all along.

**Why the wide face was wrong and not merely wide.** `renderers/feedback/sonner.tsx` passes
the value straight into `<Button variant={…}>`, whose vocabulary is exactly those six keys
of `buttonVariants`. Measured on `cva` 0.7.1, an unrecognised key contributes **no** variant
class, and `defaultVariants` applies only when the value is absent *or falsy*:

```
buttonVariants({ variant: undefined }) -> "… bg-primary text-primary-foreground …" default look
buttonVariants({ variant: 'ghost' }) -> "… hover:bg-accent …" real variant
buttonVariants({ variant: 'primary' }) -> "…" NO colour at all
buttonVariants({ variant: '' }) -> "… bg-primary …" silently 'default'
```

So the mirror was validating values the renderer visibly breaks on: `'primary'` — the
likeliest wrong spelling, since the default variant's own class is `bg-primary` — rendered a
button with no background and no text colour, and `''` was silently reinterpreted as
`default`. Nothing that renders correctly today stops validating.

**Blast radius, measured.** The key stays optional, so every published `sonner` node that
omits it keeps parsing. The two fixtures in the repo that set it
(`examples/schema-catalog/src/schemas/components-feedback-sonner/{error,promise-based-toast}.json`)
use `destructive` and `outline` — both inside the six. No consumer was found relying on a
seventh spelling.

**Model inherited, not invented.** objectui#6496 landed exactly this spelling on
`ToastSchema` for the same trigger mechanism, matched to `ButtonProps['variant']` as ground
truth. This card applies the settled shape to the sibling that still disagreed with itself.
133 changes: 127 additions & 6 deletions packages/components/src/__tests__/toast-button-variant-parity.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,15 +7,17 @@
*/

/**
* `ToastSchema['buttonVariant']` ↔ Button vocabulary parity (objectui#6496).
* `buttonVariant` ↔ Button vocabulary parity, for BOTH nodes that carry the key
* (objectui#6496 for `toast`, objectui#6541 for `sonner`).
*
* objectui#6496 declared `buttonLabel` / `buttonVariant` on `ToastSchema`, and
* the shape of the second one was the card's whole judgement call. The sibling
* it was told to copy disagrees with ITSELF — `SonnerSchema` spells
* it was told to copy disagreed with ITSELF — `SonnerSchema` spelled
* `buttonVariant` as `z.string()` in the zod mirror and as a six-member union
* in TS — so "match the sibling" picks nothing. The ground truth is what the
* in TS — so "match the sibling" picked nothing. The ground truth is what the
* value REACHES: `renderers/feedback/toast.tsx:30` passes it straight into
* `<Button variant={…}>`, so `ButtonProps['variant']` is the authority.
* `<Button variant={…}>`, so `ButtonProps['variant']` is the authority. (That
* sibling disagreement is closed by objectui#6541, below.)
*
* `@object-ui/types` has zero deps and cannot import the Button, so the
* declaration there is necessarily a hand-copied list. THIS file is what stops
Expand All@@ -29,22 +31,78 @@
* does not render "some other style", it renders a button with no background
* and no text colour. That is the concrete cost the enum buys out, and it is
* measured here rather than asserted in prose.
*
* ## objectui#6541 — `sonner` joins, and its ZOD MIRROR is pinned here too
*
* The `sonner` node carries the same two keys and reaches the same `<Button>`
* (`renderers/feedback/sonner.tsx` passes `variant={schema.buttonVariant}`).
* Its TS face had declared the six all along; its zod MIRROR was an open
* `z.string()`, so one key on one component shipped as a closed union to
* type-checkers and an open string to validators. #6541 narrowed the mirror to
* the enum — an accept-set NARROWING on a published surface, stated as such.
*
* That narrowing is pinned HERE rather than in
* `types/src/__tests__/zod-mirror-parity.test.ts`, and the reason is the reason
* the drift survived: that census compares in ONE direction — "the mirror
* accepts everything the declaration declares". A mirror WIDER than its
* declaration passes it and earns no ledger entry, so the wider-than-declared
* class is invisible across all 158 pairs. The block below measures the other
* direction — everything the mirror ACCEPTS is a value the Button actually
* draws — and it can only be measured somewhere `Button` is in scope, which
* `@object-ui/types` (zero deps, no React) by construction is not.
*
* ⛔ The one-directionality of that census is its own subject and is NOT
* touched here.
*/

import { describe, it, expect } from 'vitest';
import type { ToastSchema } from '@object-ui/types';
import type { SonnerSchema, ToastSchema } from '@object-ui/types';
import { SonnerSchema as SonnerMirror } from '@object-ui/types/zod';
import { buttonVariants, type ButtonProps } from '../ui/button';

/** The six as `ToastSchema` declares them. */
/** The six as `ToastSchema` and `SonnerSchema` both declare them. */
const DECLARED = ['default', 'secondary', 'destructive', 'outline', 'ghost', 'link'] as const;

/** A minimal `sonner` node — every key on it other than `type` is optional. */
const MINIMAL_SONNER = { type: 'sonner' } as const;

/**
* The values that make the open `z.string()` wrong, each paired with what the
* Button actually draws for it. Read as a table so the schema-side assertions
* below cannot drift away from the rendering they claim to justify.
*/
const NOT_A_VARIANT = ['primary', 'danger', 'warning', 'Default', '__not-a-variant__'] as const;

/**
* What `buttonVariants` emits for a value it does not recognise: the base
* classes and nothing else. Every "this is not a real variant" assertion below
* is measured against this string rather than against a hand-written class list.
*/
const NO_VARIANT_CLASSES = buttonVariants({ variant: '__not-a-variant__' as never });

/**
* The zod mirror's accept-set for `SonnerSchema.buttonVariant`, READ off the
* schema instead of restated next to it — a restated list is the same artefact
* the drift keeps producing.
*
* `.options` exists only on `ZodEnum`. If this key ever goes back to
* `z.string()` there is nothing to read, and this throws naming the card rather
* than comparing an empty list against an empty list and passing.
*/
function mirrorAcceptSet(): readonly string[] {
type Unwrappable = { unwrap?: () => unknown; def?: { innerType?: unknown } };
const key: unknown = SonnerMirror.shape.buttonVariant;
const inner = (key as Unwrappable).unwrap?.() ?? (key as Unwrappable).def?.innerType ?? key;
const options: unknown = (inner as { options?: unknown }).options;
if (!Array.isArray(options)) {
throw new Error(
"`SonnerSchema.buttonVariant` is not an enum in the zod mirror: it accepts more than " +
'the Button can draw (objectui#6541).',
);
}
return options as readonly string[];
}

describe('the declared `buttonVariant` vocabulary is the Button’s own', () => {
it('every declared value is a real Button variant (it contributes a class)', () => {
const inert = DECLARED.filter((v) => buttonVariants({ variant: v }) === NO_VARIANT_CLASSES);
Expand DownExpand Up@@ -111,3 +169,66 @@ describe('why the mirror is an enum and not `z.string()`', () => {
expect(buttonVariants({ variant: undefined })).not.toBe(NO_VARIANT_CLASSES);
});
});

describe('`sonner` declares the same vocabulary on BOTH of its published faces', () => {
it('the TS face matches `ButtonProps["variant"]` in BOTH directions', () => {
// Same construction as the `toast` pin above, against the same ground
// truth. This half was already true before objectui#6541 — the TS face was
// never the problem — but it is what the mirror is now pinned equal to, so
// it belongs in the same file rather than being assumed.
type Accepted = NonNullable<ButtonProps['variant']>;
type Declared = NonNullable<SonnerSchema['buttonVariant']>;

const declaredIsAccepted: Accepted = null as unknown as Declared;
const acceptedIsDeclared: Declared = null as unknown as Accepted;

expect([declaredIsAccepted, acceptedIsDeclared]).toHaveLength(2);
});

it('the ZOD MIRROR accepts exactly the six — the direction the census cannot see', () => {
// Set equality, read off the schema rather than restated. Under the old
// `z.string()` there is no `.options` to read at all, so `mirrorAcceptSet`
// throws naming the card instead of quietly comparing nothing.
expect([...mirrorAcceptSet()].sort()).toEqual([...DECLARED].sort());
});

it('the mirror accepts every value the Button actually draws', () => {
// The direction `zod-mirror-parity.test.ts` does cover, kept here so the
// pair is symmetric: a later narrowing that dropped a real look would fail
// BOTH the census and this line.
for (const value of DECLARED) {
const result = SonnerMirror.safeParse({ ...MINIMAL_SONNER, buttonVariant: value });
expect(result.success, `mirror refused real Button variant '${value}'`).toBe(true);
}
});

it('the mirror refuses the values that render colourless — measured, not assumed', () => {
// Each rejection is paired with the rendering that justifies it, in the
// same iteration: the schema refuses the value AND the Button draws no
// variant class for it. Neither half is load-bearing alone — the first
// without the second is taste, the second without the first is the bug.
for (const value of NOT_A_VARIANT) {
expect(buttonVariants({ variant: value as never }), value).toBe(NO_VARIANT_CLASSES);
const result = SonnerMirror.safeParse({ ...MINIMAL_SONNER, buttonVariant: value });
expect(result.success, `mirror accepted non-variant '${value}'`).toBe(false);
}
});

it('the mirror refuses `""` — the one wrong value that does not look wrong', () => {
// Its own pin because its rendering is the opposite of the block above:
// `cva`'s falsy fallback resolves `''` to `default`, so under the old
// `z.string()` an author could write `buttonVariant: ''`, see a correctly
// styled button, and never learn the value meant nothing.
expect(buttonVariants({ variant: '' as never })).toBe(buttonVariants({ variant: 'default' }));
expect(SonnerMirror.safeParse({ ...MINIMAL_SONNER, buttonVariant: '' }).success).toBe(false);
});

it('the key stays OPTIONAL — a bare `{ type: "sonner" }` still parses', () => {
// The narrowing is on the accept-set of the VALUE, not on requiredness.
// Every published `sonner` node that omits the key keeps parsing, and the
// two catalog fixtures that set it (`destructive`, `outline`) are inside
// the six — this is what bounds the blast radius of the narrowing.
const result = SonnerMirror.safeParse(MINIMAL_SONNER);
expect(result.success ? null : result.error.issues).toBe(null);
});
});
10 changes: 6 additions & 4 deletions packages/types/src/__tests__/toast-button-keys.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -145,11 +145,13 @@ describe('ToastSchema — the two trigger-button keys are declared (objectui#649
}
});

it('is NOT `z.string()` — the shape `SonnerSchema`’s mirror uses for the same key', () => {
it('is NOT `z.string()` — the shape `SonnerSchema`’s mirror used for the same key', () => {
// Stated as a pin because the obvious way to write this card was to copy the
// sibling mirror verbatim. Sonner's two faces disagree with each other
// (`z.string()` vs the six-member union); that disagreement is filed as
// objectui#6541 and deliberately NOT resolved here.
// sibling mirror verbatim, and at the time Sonner's two faces disagreed with
// each other (`z.string()` vs the six-member union). That disagreement was
// filed as objectui#6541 rather than fixed here, and #6541 has since
// narrowed Sonner's mirror to this same enum — so the two nodes now agree,
// and this pin keeps guarding the shape rather than the sibling.
expect(ToastSchema.safeParse({ ...MINIMAL, buttonVariant: 'anything-at-all' }).success).toBe(false);
});

Expand Down
20 changes: 16 additions & 4 deletions packages/types/src/zod/feedback.zod.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -80,9 +80,9 @@ export const ToastSchema = BaseSchema.extend({
// `buttonVariants`. `cva` contributes no variant class for an unrecognised
// key, so a non-empty string outside the six renders an unstyled button —
// and `''` is silently resolved to `default` by cva's falsy fallback
// (objectui#6496). ⚠️ `SonnerSchema` below spells the same key
// `z.string()` while its TS face declares the six-member union — that
// disagreement is filed as objectui#6541, NOT resolved here.
// (objectui#6496). `SonnerSchema` below carries the same pair of keys and
// now the same enum: it spelled `buttonVariant` as `z.string()` while its TS
// face declared the six-member union, and objectui#6541 closed that gap.
buttonLabel: z.string().optional().describe('Trigger button label'),
buttonVariant: z
.enum(['default', 'secondary', 'destructive', 'outline', 'ghost', 'link'])
Expand DownExpand Up@@ -134,7 +134,19 @@ export const SonnerSchema = BaseSchema.extend({
description: z.string().optional().describe('Toast description'),
variant: z.enum(['default', 'success', 'warning', 'error', 'info']).optional().describe('Toast variant'),
buttonLabel: z.string().optional().describe('Action button label'),
buttonVariant: z.string().optional().describe('Action button variant'),
// Narrowed from `z.string()` by objectui#6541 — an accept-set NARROWING on a
// published surface, not a widening. The reason is the one spelled out on
// `ToastSchema` above: `renderers/feedback/sonner.tsx` hands this value
// straight to `<Button variant={…}>`, whose vocabulary is the six keys of
// `buttonVariants`, and `cva` contributes no variant class outside them. The
// TS face in `../feedback.ts` has declared exactly these six all along — this
// mirror is what disagreed with it. Pinned against the Button's own
// vocabulary, in BOTH directions, in
// `components/src/__tests__/toast-button-variant-parity.test.ts`.
buttonVariant: z
.enum(['default', 'secondary', 'destructive', 'outline', 'ghost', 'link'])
.optional()
.describe('Action button variant'),
});

/**
Expand Down
Loading