feat(angular-query): add devtools theme option#10609
feat(angular-query): add devtools theme option#10609grzdev wants to merge 1 commit intoTanStack:mainfrom
Conversation
📝 WalkthroughWalkthroughTheme option support is added to Angular floating devtools. The Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
View your CI Pipeline Execution ↗ for commit feb8b3f
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/angular-query-experimental/src/devtools/with-devtools.ts (1)
138-147:⚠️ Potential issue | 🟡 MinorRemove the truthy guard when syncing
themeupdates.On line 146, the
if (theme)guard prevents updates whenthemebecomesundefined, so an existing instance retains stale theme instead of resetting to default. React and Preact implementations callsetTheme()directly without guards, and the method signature accepts optional values. Removing the guard aligns with other frameworks and allows proper synchronization when the option is removed.Suggested fix
- if (theme) devtools.setTheme(theme) + devtools.setTheme(theme)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/angular-query-experimental/src/devtools/with-devtools.ts` around lines 138 - 147, The theme update guard prevents clearing a previously set theme; instead of conditionally calling devtools.setTheme only when theme is truthy, always call devtools.setTheme(theme) in the same update block so undefined resets to default; update the code in with-devtools.ts where devtools is updated (the block that calls client && devtools.setClient(...), position && devtools.setPosition(...), etc.) to remove the if (theme) check and invoke devtools.setTheme(theme) unconditionally.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/angular-query-experimental/src/devtools/with-devtools.ts`:
- Around line 138-147: The theme update guard prevents clearing a previously set
theme; instead of conditionally calling devtools.setTheme only when theme is
truthy, always call devtools.setTheme(theme) in the same update block so
undefined resets to default; update the code in with-devtools.ts where devtools
is updated (the block that calls client && devtools.setClient(...), position &&
devtools.setPosition(...), etc.) to remove the if (theme) check and invoke
devtools.setTheme(theme) unconditionally.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9aecca21-4e5e-4db4-9062-902bd73e6a72
📒 Files selected for processing (3)
.changeset/angular-devtools-theme-option.mdpackages/angular-query-experimental/src/devtools/types.tspackages/angular-query-experimental/src/devtools/with-devtools.ts
🎯 Changes
Add
themeoption support to Angular floating devtools.React, Vue, and Solid devtools already expose the shared
theme?: Themeoption. This adds the same option to Angular floating devtools and updates an existing devtools instance withsetTheme(theme)when the option changes.This keeps Angular devtools aligned with the shared query devtools API and sibling framework bindings.
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit