feat(web): add <DataState> wrapper for RQ loading/empty/error/stale#1588
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughA new generic ChangesDataState Component & Tests
Sequence DiagramsequenceDiagram
participant Client as Client UI
participant DataState as DataState
participant Query as Query-like (isLoading/isError/data/isFetching/refetch)
participant ErrorFallback as DefaultErrorFallback / CustomError
Client->>DataState: render with query-like props + slots
DataState->>Query: read flags (isError,isLoading,isPending,isFetching,data,refetch)
alt isError or query.error
DataState->>ErrorFallback: render custom error(err, retry) or default
ErrorFallback->>Query: on retry -> refetch()
else isLoading/isPending && data == undefined
DataState->>Client: render skeleton slot
else data != undefined && isEmpty(data)
DataState->>Client: render empty slot
else data != undefined
DataState->>Client: render children(data)
alt isFetching
DataState->>Client: render stale(data, true)
end
else
DataState->>Client: render null (indeterminate)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 5/10 reviews remaining, refill in 24 minutes and 3 seconds. Comment |
Introduces a generic DataState component that unifies the four React Query states (loading, empty, error, success) into a single declarative slot API. Closes diagnostic 2026-05-03-web-deep-dive/01-frontend-ergonomics.md section 3.2 — no system-wide skeleton policy.
Skords-01
force-pushed
the
devin/1777863626-data-state
branch
from
98d54d0 to
9efccef
Compare
⏱️ CI Pipeline Duration ReportBased on the last 50 successful runs on the default branch. Overall Pipeline
Trend (last 20 runs): Per-Job Breakdown
|
9 tasks
This was referenced May 4, 2026
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
Closes the gap called out in
docs/diagnostics/2026-05-03-web-deep-dive/01-frontend-ergonomics.md§3.2 (item #5 in the roadmap, score 2.00) — there was no system-wide skeleton/empty/error/stale policy, every screen handled React Query states ad-hoc.Introduces
apps/web/src/shared/components/ui/DataState.tsx— a generic wrapper that unifies the four states every React Query hook can be in into a single declarative slot API. 4-tier precedence locked by tests:error → loading (only when data === undefined) → empty → success. Accepts a minimal RQ-shaped query (works with v4 and v5), supports customisEmptychecker, optional stale-while-revalidate slot, default skeleton + error fallback with retry. 10 contract tests cover precedence, custom matchers, and slot composition.The first refactor of high-traffic screens (
MonoTransactionsPanel,BudgetPanel,RoutineList) onto this wrapper is deferred to follow-up PRs to keep this one focused on the primitive.Governing Skill
sergeant-web-uisergeant-feature-deliveryPlaybook
Verification
pnpm lintis currently broken onmainitself due to PR #1572 bumping eslint to 10.x whileeslint-plugin-react@7.37.5still calls the removedcontext.getFilename()API. CI will surface the same TypeError on this PR until that compatibility issue is unblocked (see Reviewer Notes). No new lint violations are introduced — the failure mode is identical to the currentmain.Additional checks:
Docs and Governance
AGENTS.mdneeded an update.Updated docs:
docs/diagnostics/2026-05-03-web-deep-dive/01-frontend-ergonomics.md§3.2 — note that the wrapper is shipped + planned consumer refactors.docs/diagnostics/2026-05-03-web-deep-dive/00-overview.md— roadmap row Dashboard: Add dynamic greeting, compact cards, and safer modal sheet spacing #5 marked done with link.Risk and Rollout
Hard Rule #15
AGENTS.mdbefore coding.--no-verify.--no-verifyrationale: the local Husky pre-commit hook fails for the same reasonpnpm checkfails onmainitself —eslint-plugin-react@7.37.5is incompatible witheslint@10.3.0. This is infrastructure breakage introduced upstream by PR #1572 (Dependabot dev-deps bump), not something this PR can fix in scope. Flagging explicitly per Hard Rule #15.Reviewer Notes
check/Test coverage/Critical-flow E2Efor the same reasonmainis currently red. Suggested follow-up (separate PR): bumpeslint-plugin-reactto a release that lands jsx-eslint/eslint-plugin-react#3979, or pre-emptively migrate to@eslint-react/eslint-pluginv3+. Until then this PR cannot pass CI even though its own code is clean.docs/diagnostics/2026-05-03-web-deep-dive/00-overview.md). The other three: localStorage allowlist burndown, audit docs status table, web↔server contract tests.Summary by cubic
Adds a shared component to standardize React Query loading, empty, error, and stale states. Addresses web deep-dive §3.2 (no system-wide skeleton policy). No screens are migrated yet.
staleslot whenisFetching.isLoading/isPending,isFetching,isError,error,refetch).isEmptysupported; defaults toSkeletonCardloader and a minimal error fallback with retry.shared/components/ui/index.ts; 10 contract tests added; docs updated to mark the roadmap item done.Written for commit 9efccef. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests
Documentation