Conversation
🦋 Changeset detectedLatest commit: cd63494 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
View your CI Pipeline Execution ↗ for commit cd63494
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/devtools
@tanstack/devtools-client
@tanstack/devtools-ui
@tanstack/devtools-utils
@tanstack/devtools-vite
@tanstack/devtools-event-bus
@tanstack/devtools-event-client
@tanstack/preact-devtools
@tanstack/react-devtools
@tanstack/solid-devtools
@tanstack/vue-devtools
commit: |
cddbb15 to
2c03299
Compare
| */ | ||
| export function constructCoreClass( | ||
| importFn: () => Promise<{ default: () => JSX.Element }>, | ||
| importFn: () => Promise<{ default: (props: DevtoolProps) => JSX.Element }>, |
There was a problem hiding this comment.
nts: perhaps make a generic extending DevtoolProps
b0fb67b to
8048d9d
Compare
📝 WalkthroughWalkthroughRefactors theme architecture by renaming the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.OpenGrep is compatible with Semgrep configurations. Add an |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/devtools-utils/src/solid/class.ts (1)
26-26: Consider usingThemeTypefor consistency.The
mountmethod uses the literal type'light' | 'dark'whileDevtoolPropsusesThemeType. For consistency and maintainability, consider usingThemeTypehere as well.♻️ Suggested change
- async mount<T extends HTMLElement>(el: T, theme: 'light' | 'dark') { + async mount<T extends HTMLElement>(el: T, theme: ThemeType) {And similarly on line 68:
- async mount<T extends HTMLElement>(_el: T, _theme: 'light' | 'dark') {} + async mount<T extends HTMLElement>(_el: T, _theme: ThemeType) {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/devtools-utils/src/solid/class.ts` at line 26, The mount method and its other occurrences use the literal type 'light' | 'dark' instead of the shared ThemeType; update the signature of async mount<T extends HTMLElement>(el: T, theme: 'light' | 'dark') to use ThemeType, and replace any other literal occurrences (e.g., the usage around line 68) with ThemeType so the code consistently imports and references the same ThemeType used by DevtoolProps (ensure ThemeType is imported where needed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/cyan-times-marry.md:
- Line 6: Typo in the changeset description: locate the changeset entry whose
summary reads "Extract devtools-ui from devtools-utils to avoid theme
miss-match" and correct the word "miss-match" to "mismatch" so the summary reads
"Extract devtools-ui from devtools-utils to avoid theme mismatch".
---
Nitpick comments:
In `@packages/devtools-utils/src/solid/class.ts`:
- Line 26: The mount method and its other occurrences use the literal type
'light' | 'dark' instead of the shared ThemeType; update the signature of async
mount<T extends HTMLElement>(el: T, theme: 'light' | 'dark') to use ThemeType,
and replace any other literal occurrences (e.g., the usage around line 68) with
ThemeType so the code consistently imports and references the same ThemeType
used by DevtoolProps (ensure ThemeType is imported where needed).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f526506c-03ef-4c24-94d6-2fabe60212f9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.changeset/cyan-times-marry.mdpackages/devtools-ui/src/components/theme.tsxpackages/devtools-ui/src/index.tspackages/devtools-ui/src/styles/use-styles.tspackages/devtools-utils/bin/intent.jspackages/devtools-utils/package.jsonpackages/devtools-utils/src/solid/class-mount-impl.tsxpackages/devtools-utils/src/solid/class.ts
| '@tanstack/devtools-ui': patch | ||
| --- | ||
|
|
||
| Extract devtools-ui from devtools-utils to avoid theme miss-match |
There was a problem hiding this comment.
Typo in changeset description.
"miss-match" should be "mismatch".
-Extract devtools-ui from devtools-utils to avoid theme miss-match
+Extract devtools-ui from devtools-utils to avoid theme mismatch📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Extract devtools-ui from devtools-utils to avoid theme miss-match | |
| Extract devtools-ui from devtools-utils to avoid theme mismatch |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.changeset/cyan-times-marry.md at line 6, Typo in the changeset description:
locate the changeset entry whose summary reads "Extract devtools-ui from
devtools-utils to avoid theme miss-match" and correct the word "miss-match" to
"mismatch" so the summary reads "Extract devtools-ui from devtools-utils to
avoid theme mismatch".
Moves theme provider out of utils. Currently a mismatch of devtools and devtools utils will result in breaking an application. Relocating the theme provider to the respective devtools will stop these errors.
Summary by CodeRabbit
Bug Fixes
New Features