Restore legacy colors.blue/success and add consumer-types harness#31
Merged
Conversation
Adversarial review of the merged 0.8.x line surfaced three real follow-ups: 1. The `/legacy` shim's `colors` was missing `blue` (Tailwind sky 50–900) and `success` (#22C55E) that @policyengine/design-system 0.2.0/0.3.0 shipped. The snapshot copied into src/legacy/ for 0.8.0 came from a later workspace build that had already dropped both. Two consumer migrations (PolicyEngine/cbo-baseline-tracker#3, PolicyEngine/uk-spring-statement-2026#22) had to backfill the missing values locally — the canary that the shim wasn't a true 1:1 mirror. Restored under "Semantic colors" / "Blue accent palette" with a comment noting the migration story. 2. The migration JSDoc in src/legacy/index.ts told consumers to map `colors.gray[N]` → `palette.gray[N]` and `colors.text.warning` → `rootColorsLight['--text-warning']` without flagging that both silently change visible hex values (legacy gray is Tailwind-3 #6B7280 etc.; canonical is Slate #64748B etc. — and `--text-warning` was darkened from Mantine orange.9 to Tailwind orange-700 in 0.6.0 for WCAG AA). Annotated each pair as same-hex or value-shifting so bulk sed-replace doesn't silently regress consumers. 3. The bundler-resolution drop fixed in #29 / #30 (dist/<name>.js shadowing dist/<name>/index.d.ts under moduleResolution: "bundler") was masked because nothing in this repo type-checked the package as an external consumer. Added tests/consumer-types/ with a fixture exercising the main entry, the legacy/ subpath, and per-feature subpaths, plus a tsconfig with paths to dist/ and a vitest spawn of tsc --noEmit. CI now runs `bun run build` before `bun run test` so the harness sees fresh dist/ output. Legacy color tests pin colors.blue[50/500/600/900] and colors.success exactly so a future shim regression fails locally instead of in 18+ consumers. 295 tests pass (was 293, +2 for blue/success). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
policyengine
Bot
added
the
⚙️ Engineering...
![policyengine[bot]](https://avatars.githubusercontent.com/u/88570531?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Excellent work on this follow-up PR. All three improvements are well-implemented:
1. Legacy color restoration ✅
colors.blue(Tailwind sky 50-900) correctly restored with exact hex valuescolors.success(#22C55E) properly re-added- Test coverage is comprehensive - pins specific hex values to catch future regressions
- The test comments clearly explain the restoration history
2. Migration map annotations ✅
- Clear, detailed annotations for value-changing vs same-hex mappings
- The
colors.graynote properly warns about Tailwind-3 → Slate shift (#6B7280 → #64748B) - The
colors.text.warningnote documents the WCAG improvement (#d9480f → #c2410c, 4.30:1 → 5.18:1) - Well-formatted inline JSDoc makes it easy to scan
3. Consumer-types harness ✅
- Smart design: type-checks against built artifacts via path mappings to
dist/ - Comprehensive coverage: main entry (36 symbols), legacy subpath (4 symbols), per-feature subpaths (3 symbols)
- Proper skip behavior when
dist/is missing with actionable message - Workflow change (build before test) ensures the harness exercises fresh artifacts
- The
_LegacyColorPathsexport specifically pins.blueand.successso the exact regression that forced consumer backfills now fails CI
Code quality observations:
- Changelog fragments are clear and appropriately detailed
- Test assertions use exact hex values where appropriate
- Comments explain why (restoration history, WCAG rationale) not just what
- No security concerns, logic errors, or edge cases
This PR achieves exactly what it sets out to do - preventing silent regressions that previously required 36+ consumer PRs to fix.
policyengine
Bot
removed
the
⚙️ Engineering...
- src/legacy/index.ts: flag colors.info → semanticFills.info as a
value-shifting pair (design-system 0.3.0 had Ant blue #1890FF; 0.4.0+
and this shim use PE teal-700 #2C7A7B for brand consistency). Consumers
bumping from 0.3.x will see a one-time blue→teal shift on info usages.
Also softened the module-level JSDoc framing from 'shim mirroring
design-system' to acknowledge that some values shifted between 0.3.0
and 0.4.0 and that --text-{warning,error,success} are shim-only
accessibility variants.
- tests/consumer-types/typecheck.test.ts: switch from describe.skipIf to
an explicit it.skip with an actionable hint ("run `bun run build`")
so a dev running `bun run test` cold sees why the harness didn't
fire instead of silently missing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
Follow-up to the 0.8.x line — addresses the should-fix items from an adversarial review of the merged work.
1. Restore
colors.blueandcolors.successin the legacy shim@policyengine/design-system0.2.0/0.3.0 shipped acolors.bluepalette (Tailwind sky 50–900) and acolors.success = "#22C55E"scalar. The snapshot copied intosrc/legacy/for 0.8.0 came from a later workspace build that had already dropped both, so two consumer migrations (cbo-baseline-tracker#3, uk-spring-statement-2026#22) had to backfill them locally. Restored both with a comment noting the migration history; legacy color tests pincolors.blue[50/500/600/900]andcolors.successexactly so a future shim regression fails locally.2. Annotate value-changing pairs in the migration JSDoc
src/legacy/index.ts's migration map previously saidcolors.gray[N]→palette.gray[N]andcolors.text.warning→rootColorsLight['--text-warning']without flagging that both silently change hex values:gray[N]: legacy is Tailwind-3 (#6B7280mid), canonical is Slate (#64748Bmid)text.warning: was Mantine orange.9 (#d9480f, 4.30:1 on white — fails AA at small text), is now Tailwind orange-700 (#c2410c, 5.18:1 — passes)Annotated each pair as same-hex or value-shifting so a bulk
sed-replace from design-system to canonical names doesn't silently regress consumers.3. Add
tests/consumer-types/harnessThe bundler-resolution drop fixed in #29/#30 was masked because nothing in this repo type-checked the package as an external consumer. New harness:
tests/consumer-types/fixture.ts— imports a representative slice of the main entry, the/legacysubpath, and per-feature subpathstests/consumer-types/tsconfig.json—moduleResolution: "bundler",pathsmapped todist/tests/consumer-types/typecheck.test.ts— vitest spawnstsc --noEmitagainst the fixture and asserts exit 0bun run buildbeforebun run testso the harness sees freshly-built artifacts (without this order, the harness skips with an actionable message)Catches the silent
dist/<name>.js-shadows-dist/<name>/index.d.tsfailure in CI rather than after 36 consumer PRs.Test plan
bun run typecheckcleanbun run buildcleanbun run test— 27 files, 295 tests pass (was 293; +2 for blue/success)🤖 Generated with Claude Code