feat: refactor props passing to plugins for improved flexibility#319
Merged
AlemTuzlak merged 8 commits intomainfrom Mar 13, 2026
Merged
feat: refactor props passing to plugins for improved flexibility#319AlemTuzlak merged 8 commits intomainfrom
AlemTuzlak merged 8 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 4392d1b The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 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 4392d1b
☁️ 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: |
…ssing-to-plugins # Conflicts: # packages/devtools-utils/package.json # packages/devtools-utils/src/solid/class.test.tsx # packages/devtools-utils/src/solid/class.ts
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughRefactors plugin APIs to pass a unified Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/devtools-utils/src/react/panel.tsx (1)
33-47:⚠️ Potential issue | 🟠 MajorDon't key the mount effect off the entire
propsobject.When
propsis recreated with the same values, the effect dependency[props]still triggers because of object identity, not value equality. Sinceplugin.render()in plugins-tab.tsx creates a new props object{ theme, devtoolsOpen }on each render, the Panel component will unnecessarily unmount and remount even when neither value has changed. This causes loss of panel-local state.Depend only on the primitive values that should actually trigger a remount:
Suggested fix
- }, [props]) + }, [props.theme, props.devtoolsOpen])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/devtools-utils/src/react/panel.tsx` around lines 33 - 47, The effect that mounts CoreClass is keyed on the whole props object causing remounts when props identity changes; update the effect to depend only on the actual primitive values that should trigger a remount (e.g., destructure theme and devtoolsOpen from props and use those in the dependency array) while leaving devtools.current = new CoreClass(), devtools.current.mount(devToolRef.current, props) and devtools.current?.unmount() behavior unchanged; reference devtools.current, CoreClass, devToolRef.current, mount, and unmount so you replace [props] with a dependency array like [theme, devtoolsOpen] (or the specific primitive props used) to avoid unnecessary unmount/remounts.packages/devtools-utils/src/vue/panel.ts (1)
60-69:⚠️ Potential issue | 🟡 MinorReturn type is outdated and inconsistent with the new props structure.
The return type still references only
theme?: DevtoolsPanelProps['theme'], but the component now accepts the fullTanStackDevtoolsPluginPropsobject via thepropsprop. This creates a type mismatch between the declared return type and actual usage.♻️ Suggested fix
return [Panel, NoOpPanel] as unknown as [ DefineComponent<{ - theme?: DevtoolsPanelProps['theme'] + props?: DevtoolsPanelProps devtoolsProps: TComponentProps }>, DefineComponent<{ - theme?: DevtoolsPanelProps['theme'] + props?: DevtoolsPanelProps devtoolsProps: TComponentProps }>, ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/devtools-utils/src/vue/panel.ts` around lines 60 - 69, The declared return type for Panel and NoOpPanel is stale: it still exposes theme?: DevtoolsPanelProps['theme'] but the components actually receive the full TanStackDevtoolsPluginProps via their props; update the tuple return type to reflect the new props shape (replace the theme?: DevtoolsPanelProps['theme'] entry with props?: TanStackDevtoolsPluginProps or the exact prop name used by the components) so the DefineComponent types match the runtime signature (refer to Panel, NoOpPanel, DevtoolsPanelProps, TanStackDevtoolsPluginProps and TComponentProps to locate and align the prop types).packages/devtools-utils/src/preact/panel.tsx (1)
35-49:⚠️ Potential issue | 🟠 MajorFix effect dependency to avoid unnecessary remounts.
Line 49 uses the full
propsobject as the effect dependency. Since the cleanup function (lines 43–48) nullifiesdevtools.current, React will unmount and remount the instance whenever the parent rerenders with a fresh props object, even ifthemeanddevtoolsOpenhaven't changed. This causes panel state loss and flickering.
TanStackDevtoolsPluginPropscontains onlythemeanddevtoolsOpen, so depend on those specific properties instead:Suggested fix
- }, [props]) + }, [props.theme, props.devtoolsOpen])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/devtools-utils/src/preact/panel.tsx` around lines 35 - 49, The effect currently depends on the entire props object causing unnecessary remounts; update the useEffect dependency array in the component using useEffect, devtools.current, devToolRef.current and CoreClass to depend only on the specific TanStackDevtoolsPluginProps fields used (props.theme and props.devtoolsOpen) so the instance isn’t unmounted/recreated on unrelated parent re-renders.
🧹 Nitpick comments (4)
packages/devtools-utils/src/vue/panel.ts (1)
14-17: Consider renaming the prop to avoid confusion.Having a prop named
propsthat containsDevtoolsPanelPropsis confusing. Consider renaming topluginPropsordevtoolsPluginPropsfor clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/devtools-utils/src/vue/panel.ts` around lines 14 - 17, Rename the confusing "props" identifier and prop key to something clearer (e.g., change the top-level const name from props to pluginProps and the inner prop key from props to pluginProps) so the DevtoolsPanelProps type is exposed as pluginProps/devtoolsPluginProps; update all references/usages of the old "props" identifier and prop key (including any type annotations or destructuring) to the new name (pluginProps or devtoolsPluginProps) and ensure exports/imports reflect the rename.packages/preact-devtools/src/devtools.tsx (1)
193-232: Inconsistent parameter naming:themeshould beprops.The plugin mapping callbacks at lines 201 and 219 use
themeas the parameter name, but the actual value passed isTanStackDevtoolsPluginProps. This creates confusion since the parameter contains more than just the theme. The parameter naming should be consistent with the updated type.♻️ Suggested fix for clarity
name: typeof plugin.name === 'string' ? plugin.name - : (e, theme) => { + : (e, props) => { const id = e.getAttribute('id')! const target = e.ownerDocument.getElementById(id) if (target) { setTitleContainers((prev) => ({ ...prev, [id]: e, })) } convertRender( plugin.name as PluginRender, setTitleComponents, e, - theme, + props, ) }, - render: (e, theme) => { + render: (e, props) => { const id = e.getAttribute('id')! const target = e.ownerDocument.getElementById(id) if (target) { setPluginContainers((prev) => ({ ...prev, [id]: e, })) } - convertRender(plugin.render, setPluginComponents, e, theme) + convertRender(plugin.render, setPluginComponents, e, props) },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/preact-devtools/src/devtools.tsx` around lines 193 - 232, The callbacks inside the pluginsMap mapping use the parameter name theme but actually receive TanStackDevtoolsPluginProps; rename the parameter to props in both the custom name function (plugin.name) and the render function (plugin.render), and update all uses of that parameter inside those functions (e.g., the convertRender calls) to pass props instead of theme; ensure convertRender, setTitleComponents, setPluginComponents, setTitleContainers, and setPluginContainers usages remain unchanged except for replacing theme with props to keep naming consistent with the prop type.packages/devtools/src/tabs/index.tsx (1)
6-25: Inconsistent component signatures across tabs.Only
PluginsTabis typed to receive{ isOpen: boolean }props, butTabContentcalls all tab components with{ isOpen: props.isOpen }. While this works at runtime due to JavaScript's function arity flexibility, it creates a type inconsistency.Consider aligning the signatures for consistency:
♻️ Suggested fix for consistency
{ name: 'SEO', id: 'seo', - component: () => <SeoTab />, + component: (_props: { isOpen: boolean }) => <SeoTab />, icon: () => <PageSearch />, }, { name: 'Settings', id: 'settings', - component: () => <SettingsTab />, + component: (_props: { isOpen: boolean }) => <SettingsTab />, icon: () => <Cogs />, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/devtools/src/tabs/index.tsx` around lines 6 - 25, The tabs array has inconsistent component signatures: PluginsTab is typed as (props: { isOpen: boolean }) but SeoTab and SettingsTab are not, while TabContent invokes every tab with { isOpen: props.isOpen }; update the signatures to be consistent by ensuring every tab's component has the same type (props: { isOpen: boolean }) => JSX.Element in the tabs array and adjust SeoTab and SettingsTab (or their wrapper arrow functions in the tabs entries) to accept the props parameter (e.g., component: (props: { isOpen: boolean }) => <SeoTab {...props} />) so TypeScript sees a uniform component shape; reference symbols: tabs, PluginsTab, SeoTab, SettingsTab, TabContent.packages/react-devtools/src/devtools.tsx (1)
12-15: Document this callback signature change as a breaking API change.Line 14 changes the callback parameter shape that backs the exported
TanStackDevtoolsReactPluginsurface on Lines 24-66. Please ship this with a changeset or migration note so downstream plugins knownameandrendernow receive{ theme, devtoolsOpen }instead of a bare theme string.Also applies to: 24-66
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-devtools/src/devtools.tsx` around lines 12 - 15, The callback parameter shape for PluginRender has changed (now the render callback and plugin "name" receive an object with { theme, devtoolsOpen } instead of a bare theme string) and this is a breaking API change; add a changeset or migration note documenting the change and how downstream plugins must update their TanStackDevtoolsReactPlugin implementations (referencing PluginRender and TanStackDevtoolsReactPlugin) to accept the new object signature, update any TypeScript types/comments in packages/react-devtools/src/devtools.tsx to reflect the new parameter shape, and update documentation/examples that show using name and render to demonstrate the new { theme, devtoolsOpen } parameter format.
🤖 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/thirty-spies-fetch.md:
- Around line 2-9: Update the changeset to mark a breaking change by changing
the release type from minor to major for each listed package
('@tanstack/preact-devtools', '@tanstack/devtools-utils',
'@tanstack/react-devtools', '@tanstack/solid-devtools', '@tanstack/devtools'),
and update the summary to state the API change explicitly: the plugin callback
signature changed from (el, theme) to (el, { theme, devtoolsOpen }), so
consumers must update implementations; ensure the changeset text clearly
documents this breaking change so the packages will be released with a major
version bump.
In `@packages/devtools-utils/src/solid/class-mount-impl.tsx`:
- Around line 3-6: The import order violates the import/order lint rule because
the type import TanStackDevtoolsPluginProps from '@tanstack/devtools' is placed
before Solid runtime imports; reorder imports so Solid runtime imports (lazy,
Portal, render, and JSX) appear first and then place the type import
TanStackDevtoolsPluginProps below them (i.e., import lazy, Portal, render, and
JSX from 'solid-js'/'solid-js/web' first, then import type {
TanStackDevtoolsPluginProps } from '@tanstack/devtools') to satisfy the linter.
- Around line 10-11: The lazy importer importFn is typed as returning a zero-arg
component but at runtime you spread TanStackDevtoolsPluginProps into it; update
the importFn type so the default export is a component that accepts
TanStackDevtoolsPluginProps (e.g. Promise<{ default: (props:
TanStackDevtoolsPluginProps) => JSX.Element }> or the Solid component type for
that props shape) so TSX prop checking matches the mount code; adjust any
related casts/usages in class-mount-impl.tsx around the props/importFn
parameters (props, importFn, and the mounted component call) to use the new
typed component signature.
---
Outside diff comments:
In `@packages/devtools-utils/src/preact/panel.tsx`:
- Around line 35-49: The effect currently depends on the entire props object
causing unnecessary remounts; update the useEffect dependency array in the
component using useEffect, devtools.current, devToolRef.current and CoreClass to
depend only on the specific TanStackDevtoolsPluginProps fields used (props.theme
and props.devtoolsOpen) so the instance isn’t unmounted/recreated on unrelated
parent re-renders.
In `@packages/devtools-utils/src/react/panel.tsx`:
- Around line 33-47: The effect that mounts CoreClass is keyed on the whole
props object causing remounts when props identity changes; update the effect to
depend only on the actual primitive values that should trigger a remount (e.g.,
destructure theme and devtoolsOpen from props and use those in the dependency
array) while leaving devtools.current = new CoreClass(),
devtools.current.mount(devToolRef.current, props) and
devtools.current?.unmount() behavior unchanged; reference devtools.current,
CoreClass, devToolRef.current, mount, and unmount so you replace [props] with a
dependency array like [theme, devtoolsOpen] (or the specific primitive props
used) to avoid unnecessary unmount/remounts.
In `@packages/devtools-utils/src/vue/panel.ts`:
- Around line 60-69: The declared return type for Panel and NoOpPanel is stale:
it still exposes theme?: DevtoolsPanelProps['theme'] but the components actually
receive the full TanStackDevtoolsPluginProps via their props; update the tuple
return type to reflect the new props shape (replace the theme?:
DevtoolsPanelProps['theme'] entry with props?: TanStackDevtoolsPluginProps or
the exact prop name used by the components) so the DefineComponent types match
the runtime signature (refer to Panel, NoOpPanel, DevtoolsPanelProps,
TanStackDevtoolsPluginProps and TComponentProps to locate and align the prop
types).
---
Nitpick comments:
In `@packages/devtools-utils/src/vue/panel.ts`:
- Around line 14-17: Rename the confusing "props" identifier and prop key to
something clearer (e.g., change the top-level const name from props to
pluginProps and the inner prop key from props to pluginProps) so the
DevtoolsPanelProps type is exposed as pluginProps/devtoolsPluginProps; update
all references/usages of the old "props" identifier and prop key (including any
type annotations or destructuring) to the new name (pluginProps or
devtoolsPluginProps) and ensure exports/imports reflect the rename.
In `@packages/devtools/src/tabs/index.tsx`:
- Around line 6-25: The tabs array has inconsistent component signatures:
PluginsTab is typed as (props: { isOpen: boolean }) but SeoTab and SettingsTab
are not, while TabContent invokes every tab with { isOpen: props.isOpen };
update the signatures to be consistent by ensuring every tab's component has the
same type (props: { isOpen: boolean }) => JSX.Element in the tabs array and
adjust SeoTab and SettingsTab (or their wrapper arrow functions in the tabs
entries) to accept the props parameter (e.g., component: (props: { isOpen:
boolean }) => <SeoTab {...props} />) so TypeScript sees a uniform component
shape; reference symbols: tabs, PluginsTab, SeoTab, SettingsTab, TabContent.
In `@packages/preact-devtools/src/devtools.tsx`:
- Around line 193-232: The callbacks inside the pluginsMap mapping use the
parameter name theme but actually receive TanStackDevtoolsPluginProps; rename
the parameter to props in both the custom name function (plugin.name) and the
render function (plugin.render), and update all uses of that parameter inside
those functions (e.g., the convertRender calls) to pass props instead of theme;
ensure convertRender, setTitleComponents, setPluginComponents,
setTitleContainers, and setPluginContainers usages remain unchanged except for
replacing theme with props to keep naming consistent with the prop type.
In `@packages/react-devtools/src/devtools.tsx`:
- Around line 12-15: The callback parameter shape for PluginRender has changed
(now the render callback and plugin "name" receive an object with { theme,
devtoolsOpen } instead of a bare theme string) and this is a breaking API
change; add a changeset or migration note documenting the change and how
downstream plugins must update their TanStackDevtoolsReactPlugin implementations
(referencing PluginRender and TanStackDevtoolsReactPlugin) to accept the new
object signature, update any TypeScript types/comments in
packages/react-devtools/src/devtools.tsx to reflect the new parameter shape, and
update documentation/examples that show using name and render to demonstrate the
new { theme, devtoolsOpen } parameter format.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9fa0e804-fbdb-4cef-81cd-31840fc0cfa4
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
.changeset/thirty-spies-fetch.mdpackages/devtools-utils/package.jsonpackages/devtools-utils/src/preact/panel.tsxpackages/devtools-utils/src/preact/plugin.tsxpackages/devtools-utils/src/react/panel.tsxpackages/devtools-utils/src/react/plugin.tsxpackages/devtools-utils/src/solid/class-mount-impl.tsxpackages/devtools-utils/src/solid/class.test.tsxpackages/devtools-utils/src/solid/class.tspackages/devtools-utils/src/solid/panel.tsxpackages/devtools-utils/src/solid/plugin.tsxpackages/devtools-utils/src/vue/panel.tspackages/devtools/src/components/tab-content.tsxpackages/devtools/src/context/devtools-context.tsxpackages/devtools/src/devtools.tsxpackages/devtools/src/index.tspackages/devtools/src/tabs/index.tsxpackages/devtools/src/tabs/plugins-tab.tsxpackages/preact-devtools/src/devtools.tsxpackages/react-devtools/src/devtools.tsxpackages/solid-devtools/src/core.tsx
Comment on lines
+2
to
+9
| '@tanstack/preact-devtools': minor | ||
| '@tanstack/devtools-utils': minor | ||
| '@tanstack/react-devtools': minor | ||
| '@tanstack/solid-devtools': minor | ||
| '@tanstack/devtools': minor | ||
| --- | ||
|
|
||
| Change the way props are passed to the plugins |
There was a problem hiding this comment.
This API change needs major version bumps.
The plugin callback contract now changes from (el, theme) to (el, { theme, devtoolsOpen }), which breaks existing plugin implementations at both type level and runtime. Shipping these packages as minor will understate the break for consumers.
Suggested changeset update
-'@tanstack/preact-devtools': minor
-'@tanstack/devtools-utils': minor
-'@tanstack/react-devtools': minor
-'@tanstack/solid-devtools': minor
-'@tanstack/devtools': minor
+'@tanstack/preact-devtools': major
+'@tanstack/devtools-utils': major
+'@tanstack/react-devtools': major
+'@tanstack/solid-devtools': major
+'@tanstack/devtools': major
@@
-Change the way props are passed to the plugins
+BREAKING: plugin callbacks now receive `TanStackDevtoolsPluginProps` (`theme` + `devtoolsOpen`) instead of a theme string📝 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.
Suggested change
| '@tanstack/preact-devtools': minor | |
| '@tanstack/devtools-utils': minor | |
| '@tanstack/react-devtools': minor | |
| '@tanstack/solid-devtools': minor | |
| '@tanstack/devtools': minor | |
| --- | |
| Change the way props are passed to the plugins | |
| '@tanstack/preact-devtools': major | |
| '@tanstack/devtools-utils': major | |
| '@tanstack/react-devtools': major | |
| '@tanstack/solid-devtools': major | |
| '@tanstack/devtools': major | |
| --- | |
| BREAKING: plugin callbacks now receive `TanStackDevtoolsPluginProps` (`theme` + `devtoolsOpen`) instead of a theme string |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.changeset/thirty-spies-fetch.md around lines 2 - 9, Update the changeset to
mark a breaking change by changing the release type from minor to major for each
listed package ('@tanstack/preact-devtools', '@tanstack/devtools-utils',
'@tanstack/react-devtools', '@tanstack/solid-devtools', '@tanstack/devtools'),
and update the summary to state the API change explicitly: the plugin callback
signature changed from (el, theme) to (el, { theme, devtoolsOpen }), so
consumers must update implementations; ensure the changeset text clearly
documents this breaking change so the packages will be released with a major
version bump.
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.
🎯 Changes
✅ Checklist
pnpm test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Breaking Changes
Tests
Chores