chore(web-ui): frontend major-dependency upgrades — Jest 30, jsdom 30, ESLint 10, Tailwind 4 (#658)#695
Conversation
- Bump jest, @types/jest, jest-environment-jsdom to ^30. - jsdom 30 makes window.location non-configurable; add a small src/lib/navigation seam (currentPathname/redirectTo) and route the api.ts 401 redirect and auth.ts logout through it so redirects stay mockable. api.auth.test mocks the seam instead of stubbing location. - Jest 30 dropped json-summary from default coverageReporters; set them explicitly so CI's coverage-summary.json gate keeps working. Refs #658
- Bump eslint to ^10, typescript-eslint to ^8.61 and eslint-plugin-react-hooks to ^7.1 (all support eslint ^10). - eslint-plugin-react has no eslint-10 release and contributed only a no-op 'react/react-in-jsx-scope: off' rule (never enabled by the next or recommended configs), so drop it plus the unused eslint-config-next dependency. Lint output is unchanged (0 errors, same 3 pre-existing exhaustive-deps warnings). Refs #658
- Bump tailwindcss to ^4, add @tailwindcss/postcss, drop autoprefixer (v4 vendors prefixes automatically). - postcss.config.js uses the @tailwindcss/postcss plugin. - globals.css: replace @tailwind directives with @import 'tailwindcss' and load the existing JS theme via @config (no CSS-first rewrite). - tailwind.config.ts: darkMode 'class' (v4 dropped the single-element array form). Build emits all custom theme utilities, the .dark variant, and the typography plugin. - Apply v4 size-scale utility renames to preserve appearance/a11y: outline-none->outline-hidden (forced-colors focus behavior), shadow-sm->shadow-xs, rounded-sm->rounded-xs, blur-sm->blur-xs. Refs #658
|
Warning Review limit reached
More reviews will be available in 3 hours, 22 minutes, and 16 seconds. Learn how PR review limits work. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (43)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…logout test - Tailwind v4 has no bare 'shadow' utility (only shadow-xs..2xl), so the card/button/badge/checkbox/tabs 'shadow' classes silently rendered no shadow. Map them to 'shadow-sm' (v4's equivalent of v3's default shadow: 0 1px 3px). Caught by a built-CSS audit, not jsdom/build. - auth.test.ts: mock the @/lib/navigation seam so logout()'s redirect is asserted without jsdom 30's 'Not implemented: navigation' console noise (codex review feedback). Refs #658
|
Code Review -- PR 695: Frontend major-dependency upgrades (Jest 30, ESLint 10, Tailwind 4) Overall: LGTM. Clean, well-scoped upgrade. Each migration follows the correct path and the PR description explains every breaking-change decision clearly. What is done well: Navigation seam (src/lib/navigation.ts): The indirection layer for window.location is the right call for jsdom 30. It is SSR-safe, tiny surface area, and makes redirect behavior mockable without brittle DOM hacks. The test rewrite in api.auth.test.ts -- swapping Object.defineProperty(window, location) for jest.mock -- is strictly cleaner and will not break on future jsdom changes. coverageReporters fix in jest.config.js: Explicitly requesting json-summary is a critical guard. CI coverage threshold gate would silently fail to read coverage/coverage-summary.json without it. Tailwind v4 renames (complete sweep): Verified no outline-none, shadow-sm, rounded-sm, or blur-sm appear in any added lines. Sweep covers button, input, checkbox, badge, select, textarea, tabs, and page components. The backdrop-blur-sm to backdrop-blur-xs rename in DiffViewer is caught too. ESLint 10 cleanup: eslint-config-next was never imported in the flat config, and the lint script runs eslint directly (not next lint). Removing it is correct. Dropping eslint-plugin-react is safe -- its react/react-in-jsx-scope:off rule was a no-op under Next.js recommended configs. darkMode: class (not array): The array form was deprecated in Tailwind v4 with the @config bridge. Fixed correctly. Minor observations (non-blocking):
Summary: All three upgrades are implemented correctly. The navigation seam is the most architecturally interesting addition and it is solid. Ready to approve once a quick visual sanity check of the Tailwind-renamed components is done. |
Verification & review cycleLocal gates (all green):
Demo (Tailwind v4 — the risky part): built the app, served Review cycle (cross-family + CSS audit):
Known limitation: CI has no CSS visual-diff. The v4 utility renames here preserve appearance, but a manual visual pass remains the only thing that catches subtle theme shifts. |
Code Review — PR #695: Frontend Major Dependency UpgradesThis is a well-prepared migration PR. The three upgrade areas are clearly separated and the PR description is exemplary — every non-obvious decision has a documented rationale. No prior reviews to reconcile against; this is a first pass. OverviewUpgrades Jest 29→30, jsdom 29→30, ESLint 9→10, and Tailwind 3→4. The PR is cleanly structured as three logical commits, each independently passing CI. The migration work is correct and the known limitations are honestly documented. What is done well
Tailwind v4 utility renames are consistently applied across all UI components: Test updates in Issues and observations1. Node.js version precision (low risk) ESLint 10 transitive deps ( 2.
3.
4.
Minor nits
VerdictNo blocking issues. Migration is correct, 1020 tests pass at 68.91% coverage (above the 65% gate), and all CI gates remain intact. The Node engine precision note and the Approve when CI is green. Once merged, close the superseded Dependabot PRs #639, #640, #641, #642 as noted in the PR description. |
ESLint 10's transitive deps require Node ^20.19 || ^22.13 || >=24. The floating '20' tag currently resolves to a new enough patch, but pinning the minimum makes the requirement explicit and regression-proof. Refs #658
|
Thanks — addressed the review notes:
CI is re-running on the pin; will merge once green. |
Code Review — PR 695 Follow-up (post NODE_VERSION pin)Reviewing the latest commit (b0f7692) and confirming disposition of all prior open items. NODE_VERSION pin — correct NODE_VERSION: '20.19' in test.yml is the right fix. The ESLint 10 dep tree (@eslint/config-array, @eslint/core, etc.) declares node: '^20.19.0 || ^22.13.0 || >=24' — a floating '20' tag resolves to whatever the runner images have cached and could silently land on a 20.18.x build. The pin is minimal (only locks the patch floor, not a full version triple) and will automatically ride forward with npm's SemVer resolution as runner images update. Prior open items — resolved
Nothing new was introduced in the NODE_VERSION commit. Final verdict All review items are closed. The three upgrade areas (Jest 30, ESLint 10, Tailwind 4) are correctly implemented, 1020 tests pass at 68.91% coverage (above the 65% gate), and CI gates remain intact. Ready to merge. Close superseded Dependabot PRs 639, 640, 641, 642 once landed. |
1 participant
Summary
Closes #658 — the four held-out major-version frontend dependency upgrades. Blocker #646 (CI now runs
npm run build+ frontend lint) is merged, so all four are verifiable in CI.Done as one branch, three commits, each independently green:
jest,@types/jest,jest-environment-jsdom→ 30eslint→ 10tailwindcss→ 4 (CSS-first PostCSS pipeline)Notable migration work
Jest 30 / jsdom 30 — jsdom 30 makes
window.locationnon-configurable, so the oldObject.defineProperty(window, 'location', …)test stub throws. Added a tinysrc/lib/navigation.tsseam (currentPathname/redirectTo); theapi.ts401 redirect andauth.tslogout route through it, andapi.auth.test.tsmocks the seam instead of pokingwindow.location. Also re-addedjson-summarytocoverageReporters(Jest 30 dropped it from defaults) so CI's coverage-summary gate keeps working.ESLint 10 — bumped
typescript-eslint(8.61) andeslint-plugin-react-hooks(7.1) to their eslint-10-supporting versions.eslint-plugin-reacthas no eslint-10 release and contributed only a no-opreact/react-in-jsx-scope: offrule (never enabled by the next/recommended configs), so it and the unusedeslint-config-nextdep were removed. Lint output is unchanged (0 errors, same 3 pre-existingexhaustive-depswarnings).Tailwind 4 —
@tailwinddirectives →@import 'tailwindcss'+@configloading the existing JS theme (no CSS-first rewrite);@tailwindcss/postcssreplaces the integrated plugin;autoprefixerdropped (v4 vendors prefixes).darkMode: 'class'(v4 dropped the single-element-array form). Applied v4 size-scale renames to preserve appearance/a11y:outline-none→outline-hidden,shadow-sm→shadow-xs,rounded-sm→rounded-xs,blur-sm→blur-xs.Verification
npm run build✅ — emits all custom theme utilities, the.darkvariant, the typography.proseplugin, and the renamed utilities.npm run test:coverage✅ — 1020 tests pass, coverage 68.91% (gate 65%).npm run lint✅ — 0 errors (3 pre-existing warnings, unchanged from baseline).Known limitations
next buildvalidates compilation, not pixels. The Tailwind v4 utility renames above were applied to preserve the v3 appearance, but a manual visual pass is still the only thing that would catch subtle theme shifts.@config(deliberate — minimal diff). A future CSS-first@thememigration is optional.Follow-up
Once merged, the superseded Dependabot PRs #639, #640, #641, #642 should be closed.