fix(react-doctor): migrate components to React 19 APIs#347
Conversation
bntvllnt
added
react-doctor
bntvllnt
left a comment
bntvllnt
left a comment
There was a problem hiding this comment.
Blocking review note (GitHub would not allow REQUEST_CHANGES because this appears to be authored by the same account): this PR raises @vllnt/ui's React peer floor to React 19 and migrates the public component implementation/API surface, but it does not update packages/ui/CHANGELOG.md. The repo PR playbook requires user-facing changes to add an [Unreleased] changelog entry, and this is especially important because React 18 consumers will be excluded by the new peer dependency.
Observed gates: PR is linked to #268, mergeable/clean, and GitHub checks were green when queried (Quality Gates, build, Storybook, Storybook Tests, Verify Stories, Visual Regression, Vercel previews). I did not find another code-level blocker in the sampled React 19 migration patterns.
| "next-themes": ">=0.4.0", | ||
| "react": ">=18.0.0", | ||
| "react-dom": ">=18.0.0", | ||
| "react": ">=19.0.0", |
There was a problem hiding this comment.
Blocking: this raises the public React peer floor from React 18 to React 19, but packages/ui/CHANGELOG.md was not changed. The repo playbook requires user-facing changes to add an [Unreleased] changelog entry; please document this under Changed (and call out that React 19 is now required) before merge.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
bntvllnt
left a comment
bntvllnt
left a comment
There was a problem hiding this comment.
Review — CodeQL remediation head check
Head reviewed: 1f5496561043225e9e79b4104142a6f1cf48b79f
Scope of this pass: delta since prior reviewed head f854c2199efad3c1cf190f32fedca1f8a89d4352, plus PR body and live status rollup.
BLOCKING
- None found in the source changes reviewed for this remediation delta.
WARN / OPERATIONAL
Vercel – ui.vllnt.aiis still failing on the current head, but the live status description isCanceled from the Vercel Dashboard:- https://vercel.com/vllnt/ui.vllnt.ai/72ZCPuydsBHkEUJtdNWM7Ed71mHh
- I do not see this as a source-level CodeQL/GitHub Actions failure. It still needs a Vercel dashboard redeploy/rerun or a fresh deployment trigger, then monitoring to green before merge readiness can be called clean.
VERIFIED CLEAN
- The only files changed since the prior reviewed head are:
packages/ui/src/components/ai-artifact/ai-artifact.tsxpackages/ui/src/components/kbd/kbd.tsx
AIArtifactreplaced the CodeQL-flagged slug regexes with bounded character iteration and preserves the previous behavior: lowercase ASCII letters/digits are kept, invalid runs become separators, leading/trailing separators are dropped, and empty titles fall back toartifact.Kbdreplaced the shortcut separator regex with.split("+").map(trim).filter(Boolean), preservingmod+k/ spaced shortcut tokenization without a polynomial regex path.- PR body matches the current head for this remediation: it names
1f5496561043225e9e79b4104142a6f1cf48b79f, includes the CodeQL remediation spot-checks, keepsCloses #268, and documents the canceled Vercel status. - Live checks at current head: CodeQL, Quality Gates, react-doctor, PR Issue Link, Storybook build/tests/visual/verify, Vercel Preview Comments, and
Vercel – storybookare passing.
VALIDATION
- Ran locally in the detached review checkout:
pnpm -F @vllnt/ui exec eslint src/components/ai-artifact/ai-artifact.tsx src/components/kbd/kbd.tsx— passpnpm -F @vllnt/ui exec tsc --noEmit --project tsconfig.build.json— pass
- Re-checked live GitHub status rollup and commit statuses at current head.
Verdict: CodeQL remediation is clean. Do not treat the remaining Vercel – ui.vllnt.ai status as a code blocker, but it still needs redeploy/rerun + monitoring before final merge readiness.
bntvllnt
left a comment
bntvllnt
left a comment
There was a problem hiding this comment.
@bntvllnt Review verdict: APPROVE recommended for reviewer triage — stale Vercel waived by PM decision
Head reviewed: 1f5496561043225e9e79b4104142a6f1cf48b79f
I re-checked the current status against the PM waiver in pm-decision.md: all GitHub Actions/code checks are green and Vercel – storybook is SUCCESS. The remaining Vercel – ui.vllnt.ai: FAILURE matches the accepted stale external UI preview status for reviewer triage only.
No new source-level blockers found beyond the prior current-head CodeQL remediation review. This is not merge/release approval and does not weaken branch protection; it only records that the stale UI preview status is waived for review triage under the PM decision.
|
Preview ready · pr-347-ui-registry
Inspect
|
…p max-lines suppressions
- Replace forbidden `as` cast on FormBase with Object.assign(FormInner, { displayName: "Form" }) in both packages/ui and registry copies, preserving generic type parameter
- Extract SparklineItem sub-component from SparklineGrid render callback (packages/ui + registry)
- Extract ThreadBubbleHeader sub-component from ThreadBubble (packages/ui + registry)
- Extract useRegionSelection hook from ChoroplethMap (packages/ui + registry)
- Move zoom useState into useToolbarHandlers hook in InteractiveTimeline (packages/ui + registry)
- Replace single-use resolvedLabels useMemo with inline expression in GeographyQuizMap and TreeView (packages/ui + registry)
- All max-lines-per-function eslint-disable suppressions removed
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
bntvllnt
left a comment
bntvllnt
left a comment
There was a problem hiding this comment.
@bntvllnt Review verdict: COMMENT — manual approval recommended; final approval reserved for bntvllnt.
Head reviewed: c99d612d818405161cef0b1c67148798485eae41
BLOCKING
None remaining.
The earlier same-head metadata blocker is resolved: the PR body now lists Current head: c99d612d818405161cef0b1c67148798485eae41, includes current-head validation evidence, and the live head still matches that SHA.
WARN
None.
VERIFIED CLEAN
- Full changed-file review coverage is complete for the current head. The PR remains a React 19 migration/remediation:
forwardRef-> ref-as-prop components,useContext->use(), React peer dependency/changelog updates, registry regeneration, and the later react-doctor/CodeQL cleanup deltas. - The current-head remediation delta remains clean: FormBase avoids the forbidden cast via
Object.assign, max-lines suppressions were removed through extracted helpers/components, and regenerated registry copies match the source direction. - Live GitHub checks are green at the current head;
mergeStateStatusisCLEAN. - PR metadata now matches the reviewed head and validation evidence.
VALIDATION
- Live prepublish check: PR #347 is OPEN, non-draft,
mergeStateStatus=CLEAN, headc99d612d818405161cef0b1c67148798485eae41. - Live GitHub checks: passing (
gh pr checks 347 --repo vllnt/ui, exit 0). - Current PR body was re-fetched and verified to contain the current head SHA and current-head validation section.
- Prior current-head review artifact on this same head recorded the local validation:
git diff --check origin/main...HEAD, targeted@vllnt/uiESLint on the remediation files, andpnpm -F @vllnt/ui exec tsc --noEmit --project tsconfig.build.jsonall passed.
No automatic approval was submitted.
bntvllnt
left a comment
bntvllnt
left a comment
There was a problem hiding this comment.
Review follow-up — 1 blocking operational finding
Head reviewed: c99d612d818405161cef0b1c67148798485eae41
BLOCKING
C1 — preview deploy check is green but the published preview endpoint returns 503- Evidence:
gh pr checks 347 --repo vllnt/uireportsvllnt-pilot / preview deployasSUCCESSand links tohttps://pr-347-ui-registry.preview.vllnt.ai; direct probe of that link returnsHTTP/2 503with bodyno available server. - Why it matters: the PR body says the live status rollup is passing, including the preview deploy, but the user-facing preview artifact is not actually reachable. That makes the validation evidence incomplete for a UI/registry migration until the preview route is healthy or explicitly waived by the human owner.
- Fix: restore the PR preview route/deployment for this head, update the PR body with the re-check evidence, or get an explicit PM/owner waiver if preview access is not required for this PR.
- Evidence:
WARN
None.
VERIFIED CLEAN
- PR is open and non-draft at the requested head.
- PR links issue
#268and the body records the current head SHA. - Live GitHub checks are green at the current head: Quality Gates, CodeQL, issue-link enforcement, Storybook build/tests/visual/verify, registry/storybook deploy jobs, and the preview deploy check all report
SUCCESS. - Local
git diff --check origin/main...HEADpassed. - Changed-file coverage was reviewed at the gate level: 330 changed paths total, including 172
packages/uipaths, 157apps/registrypaths, andpnpm-lock.yaml. The React 19 migration pattern removes changed-fileforwardRef/useContextmatches and raises React/React DOM peers to>=19.0.0with a changelog entry.
VALIDATION
- Ran:
gh pr view 347 --repo vllnt/ui --json ... - Ran:
gh pr checks 347 --repo vllnt/ui --json name,state,workflow,startedAt,completedAt,link,bucket - Ran:
git diff --check origin/main...HEAD - Ran:
curl -sS -I -L --max-time 20 https://pr-347-ui-registry.preview.vllnt.ai - Ran:
curl -sS -L --max-time 20 https://pr-347-ui-registry.preview.vllnt.ai - Not run locally: full
pnpmworkspace gates; relied on current-head PR body evidence and green GitHub checks for those.
Review verdict: COMMENT with blocking recommendation; final formal approval/request-changes remains reserved for bntvllnt.
|
Preview availability gate update (Hermes task
Conclusion: the GitHub check/deployer response is not sufficient preview-health evidence for this PR right now. The PR should remain blocked on preview availability until either the preview route/server is restored and the URL returns a real page, or an explicit PM/owner waiver records that preview availability is not required for this PR. |
2 participants
Summary
Validation
Current head:
c99d612d818405161cef0b1c67148798485eae41.Current-head delta validation recorded for
c99d612d818405161cef0b1c67148798485eae41:git diff --check origin/main...HEAD— pass.pnpm -F @vllnt/ui exec eslint src/components/form/form.tsx src/components/choropleth-map/choropleth-map.tsx src/components/geography-quiz-map/geography-quiz-map.tsx src/components/interactive-timeline/interactive-timeline.tsx src/components/sparkline-grid/sparkline-grid.tsx src/components/thread-bubble/thread-bubble.tsx src/components/tree-view/tree-view.tsx— pass.pnpm -F @vllnt/ui exec tsc --noEmit --project tsconfig.build.json— pass.Full React 19 migration validation from the PR history:
pnpm -F @vllnt/ui lint— pass.pnpm -F @vllnt/ui exec tsc --noEmit --project tsconfig.build.json— pass.pnpm -F @vllnt/ui-registry exec tsc --noEmit --project tsconfig.json— pass.pnpm build— pass.pnpm test:once— 216 files, 1215 tests passed.npx --yes react-doctor . --offline --json—no-react19-deprecated-apis=0.npx --yes react-doctor . --diff origin/main --annotations --fail-on error --offline— pass.f854c2199efad3c1cf190f32fedca1f8a89d4352:git diff --check -- packages/ui/CHANGELOG.md,pnpm exec prettier --check packages/ui/CHANGELOG.md, and a script assertion that the React 19 peer-requirement entry is under[Unreleased]->Changedall passed.1f5496561043225e9e79b4104142a6f1cf48b79f:pnpm -F @vllnt/ui exec eslint src/components/ai-artifact/ai-artifact.tsx src/components/kbd/kbd.tsxpassed.1f5496561043225e9e79b4104142a6f1cf48b79f:pnpm -F @vllnt/ui exec tsc --noEmit --project tsconfig.build.jsonpassed.CI
c99d612d818405161cef0b1c67148798485eae41.build · sign · scan · deploy (storybook),build · sign · scan · deploy (ui-registry), andvllnt-pilot / preview deployare all SUCCESS.mergeStateStatus: CLEAN.Closes #268