refactor(state): delete the useApp() merged view and migrate tests to domain hooks - #418
Open
NesiciCoding wants to merge 1 commit into
Open
refactor(state): delete the useApp() merged view and migrate tests to domain hooks#418NesiciCoding wants to merge 1 commit into
NesiciCoding wants to merge 1 commit into
Conversation
… domain hooks
useApp() merged every domain context into one value, so a single dispatch
re-rendered all of its consumers — the exact whole-app re-render the domain
split and selector store exist to prevent. App code was already migrated to
the domain hooks (useRoster, useAuthoring, ...), leaving useApp() as a
regression trap with no remaining production call sites.
Delete the hook and its AppContextValue-only import, and migrate the test
suite off it:
- Context tests (AppContext, AppContext.extended, dyslexiaMode,
themeBundleApply) now render each case against exactly the domain hooks it
exercises, e.g. renderHook(() => useRoster()) or a two-hook composition
like ({ ...useRoster(), ...useAuthoring() }) — which also makes each test's
domain dependency explicit instead of silently subscribing to all seven.
- All 57 shared vi.mock('.../AppContext') factories in page/component suites
drop the now-nonexistent useApp export; their domain-hook mocks are
unchanged.
- Comments referencing useApp() updated to describe the domain hooks.
The domain-hook imports in AppContext.tsx are narrowed to just the Provider
components (the hooks are re-exported separately), removing the last unused
imports. Verified: tsc --noEmit clean, eslint 0 errors, and all 2,863 tests
pass with zero useApp references remaining anywhere in src.
Generated with Codebuff 🤖
Co-Authored-By: Codebuff <noreply@codebuff.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (62)
💤 Files with no reviewable changes (48)
📝 WalkthroughWalkthroughThe merged ChangesAppContext hook migration
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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.
What
Deletes
useApp()— the merged view that spread all seven domain contexts into one value — and migrates every remaining test call site to the domain hooks.AppContext.tsx: the hook and its only import consumer (AppContextValue) are gone; domain-hook imports narrow to just the Provider components (the hooks are re-exported separately).AppContext.test.tsx,AppContext.extended.test.tsx,dyslexiaMode.test.tsx,themeBundleApply.test.tsx): eachrenderHook(() => useApp(), ...)becomesrenderHook(() => useRoster(), ...)or a small composition like({ ...useRoster(), ...useAuthoring() })covering exactly the domains that test exercises.vi.mock('.../AppContext')factories in page/component suites: the now-nonexistentuseAppexport is dropped; the domain-hook mocks are untouched.useApp()updated.Why
useApp()merged every domain context, so a single dispatch re-rendered all of its consumers — the exact whole-app re-render the domain split and the selector store exist to prevent. Zero production call sites remained (the #414 ESLint rule enforced that), so the hook was pure regression-trap surface. Deleting it forces every consumer — including tests — to name the domain(s) they actually use.Verification
tsc --noEmitclean,npm run lint0 errors (159 warnings, at/below the pre-change baseline).grep -rn "useApp" src→ 0 matches.Summary by CodeRabbit
Refactor
Tests