-
Notifications
You must be signed in to change notification settings - Fork 515
refactor(store): Extract utility functions into store/utils/ #708
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Summary of ChangesHello @Shironex, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the application's store by reorganizing utility functions into a more modular and maintainable structure. By extracting theme, shortcut, and usage-related logic into their own dedicated files, the change enhances code clarity and separation of concerns within the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis PR moves theme, shortcut, and Claude-usage helpers out of store type files into new utility modules, adds a utils barrel export, updates imports/exports in the store and electron type import paths, and re-exports select utilities from the new modules. No runtime behavior changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request does a great job of refactoring pure utility functions out of app-store.ts into dedicated modules under store/utils/. This significantly improves the organization and maintainability of the store logic. The changes are well-structured, and maintaining backward compatibility through re-exports is a good approach. I have a couple of suggestions for the new theme-utils.ts file to further improve encapsulation and consistency with your storage abstraction.
eda187d to
2765c1f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/ui/src/store/utils/theme-utils.ts`:
- Around line 48-63: The getEffectiveFont function currently returns null when a
provided projectFont is invalid, which prevents a valid globalFont from being
used; update getEffectiveFont (and its isValidFont check) so that an invalid
projectFont is treated as "unset" (i.e., do not return null from the projectFont
branch) and the logic falls through to evaluate globalFont instead, preserving
the existing DEFAULT_FONT_VALUE handling and the final return behavior that maps
DEFAULT_FONT_VALUE to null; locate getEffectiveFont and isValidFont in
theme-utils.ts and remove the early return on invalid projectFont so the
globalFont fallback can apply.
- Around line 90-105: The direct localStorage.removeItem calls in
saveFontSansToStorage and saveFontMonoToStorage are unsafe for SSR/tests;
replace those with the storage wrapper's remove method (use the same helper
namespace as setItem, e.g., removeItem(FONT_SANS_STORAGE_KEY) and
removeItem(FONT_MONO_STORAGE_KEY)) or guard access via the helper so
reads/writes/removals all go through the storage utility rather than calling
localStorage directly.
Move pure utility functions from app-store.ts and type files into dedicated utils modules for better separation of concerns: - theme-utils.ts: Theme and font storage utilities - shortcut-utils.ts: Keyboard shortcut parsing/formatting - usage-utils.ts: Usage limit checking All utilities are re-exported from store/utils/index.ts and app-store.ts maintains backward compatibility for existing imports. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2765c1f to
8caec15
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/ui/src/store/utils/shortcut-utils.ts`:
- Around line 31-76: formatShortcut currently collapses cmdCtrl into
platform-specific labels so Windows/Linux show "Win"/"Super" for Ctrl shortcuts;
change the mapping so non-mac platforms use "Ctrl" (and the Ctrl symbol/text)
instead of "Win"/"Super". In formatShortcut, update the cmdCtrl branch and its
forDisplay/non-forDisplay branches (the code that pushes platform === 'darwin' ?
'⌘' : platform === 'win32' ? '⊞' : '◆' and the corresponding
'Cmd'/'Win'/'Super') to push 'Ctrl' (or the appropriate display symbol/text for
Ctrl) when platform !== 'darwin', ensuring both display-mode (forDisplay) and
non-display mode use Ctrl for non-mac instead of Win/Super.
🧹 Nitpick comments (2)
apps/ui/src/store/utils/shortcut-utils.ts (1)
1-1: Prefer a shared-package or canonical barrel for these types.This new relative import adds another deep path into store types; if these are core UI types, consider moving them to a shared package (e.g.,
@automaker/types) or at least importing from the store types barrel to keep paths stable.As per coding guidelines, and based on learnings, ...
apps/ui/src/store/app-store.ts (1)
102-121: Align new utils imports with the shared‑package import policy.These new relative imports introduce additional internal paths; if the utilities are intended for reuse across modules, expose them via an
@automaker/* package (or a canonical barrel) and import from there to match the repo’s import convention.As per coding guidelines, ...
Summary
app-store.tsinto a modularstore/utils/directoryapp-store.tsChanges
New Files Created (
apps/ui/src/store/utils/)theme-utils.tsshortcut-utils.tsusage-utils.tsindex.tsFiles Modified
apps/ui/src/store/app-store.ts./utils, remove inline definitions, update re-exportsapps/ui/src/store/types/ui-types.tsapps/ui/src/store/types/usage-types.tsapps/ui/src/lib/electron.ts@/store/app-storeinstead of@/store/types/usage-typesBackward Compatibility
All existing imports from
@/store/app-storecontinue to work:New canonical import path:
Test Plan
npm run build)npm run lint- 0 errors)npm run test:packages- 519 tests)npm run test:server- 1,415 tests)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.