feat: add angular devtools support and fix core devtools reactivity#6248
Open
riccardoperra wants to merge 10 commits into
Open
feat: add angular devtools support and fix core devtools reactivity#6248riccardoperra wants to merge 10 commits into
riccardoperra wants to merge 10 commits into
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR introduces TanStack Table Devtools across Angular and React frameworks by creating a new Angular devtools package, integrating devtools into 20+ Angular examples, adding React table instance ID tracking, enhancing React devtools, and refactoring the core table-devtools module to use Solid reactivity. ChangesAngular Table Devtools Package and Examples
React Table Devtools Support
Table Devtools Solid Reactivity Migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
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 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 |
riccardoperra
changed the title
|
View your CI Pipeline Execution ↗ for commit 8a240a5
☁️ Nx Cloud last updated this comment at |
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
At least one additional CI pipeline execution has run since the conclusion below was written and it may no longer be applicable.
Nx Cloud is proposing a fix for your failed CI:
We resolved two CI failures introduced by the angular devtools integration. The sherif failure was fixed by reordering dependencies alphabetically in the composable-tables example, and the knip failures were fixed by removing the unused @tanstack/angular-devtools dependency from the package, adding the missing tslib dependency required by importHelpers, and removing the export keyword from the internal-only generateId function.
Tip
✅ We verified this fix by re-running table:test:sherif, table:test:knip.
Suggested Fix changes
diff --git a/examples/angular/composable-tables/package.json b/examples/angular/composable-tables/package.json
index 2663831d..3d441290 100644
--- a/examples/angular/composable-tables/package.json
+++ b/examples/angular/composable-tables/package.json
@@ -19,8 +19,8 @@
"@angular/router": "^21.2.11",
"@faker-js/faker": "^10.4.0",
"@tanstack/angular-devtools": "https://pkg.pr.new/TanStack/devtools/@tanstack/angular-devtools@368",
- "@tanstack/angular-table-devtools": "^9.0.0-alpha.43",
"@tanstack/angular-table": "^9.0.0-alpha.45",
+ "@tanstack/angular-table-devtools": "^9.0.0-alpha.43",
"rxjs": "~7.8.2",
"tslib": "^2.8.1"
},
diff --git a/packages/angular-table-devtools/package.json b/packages/angular-table-devtools/package.json
index 7bf1bf22..2df42e78 100644
--- a/packages/angular-table-devtools/package.json
+++ b/packages/angular-table-devtools/package.json
@@ -44,10 +44,10 @@
"src"
],
"dependencies": {
- "@tanstack/angular-devtools": "https://pkg.pr.new/TanStack/devtools/@tanstack/angular-devtools@368",
"@tanstack/devtools-utils": "https://pkg.pr.new/TanStack/devtools/@tanstack/devtools-utils@368",
"@tanstack/table-core": "workspace:*",
- "@tanstack/table-devtools": "workspace:*"
+ "@tanstack/table-devtools": "workspace:*",
+ "tslib": "^2.8.1"
},
"peerDependencies": {
"@angular/core": ">=21.0.0"
diff --git a/packages/angular-table-devtools/src/injectTanStackTableDevtools.ts b/packages/angular-table-devtools/src/injectTanStackTableDevtools.ts
index 40303e30..e769ae3b 100644
--- a/packages/angular-table-devtools/src/injectTanStackTableDevtools.ts
+++ b/packages/angular-table-devtools/src/injectTanStackTableDevtools.ts
@@ -24,7 +24,7 @@ function normalizeName(name?: string) {
}
let autoId = 0
-export function generateId(): string {
+function generateId(): string {
const appId = inject(APP_ID)
return `tanstacktable-${appId}_${autoId++}${Date.now().toString(36)}`
}
Or Apply changes locally with:
npx nx-cloud apply-locally DsIG-vU7o
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
riccardoperra
force-pushed
the
feat/angular-devtools
branch
2 times, most recently
from
9a3e490 to
d397a99
Compare
riccardoperra
changed the title
riccardoperra
force-pushed
the
feat/angular-devtools
branch
2 times, most recently
from
5b002f3 to
926dcf9
Compare
riccardoperra
changed the title
riccardoperra
force-pushed
the
feat/angular-devtools
branch
from
17bc739 to
cd854d6
Compare
Refactor devtools panel with a more solid-idiomatic approach, using memo and dynamic `useTableState` subscription via Accessor getter to not break reactivity.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
riccardoperra
force-pushed
the
feat/angular-devtools
branch
from
2967e10 to
20b40f9
Compare
riccardoperra
force-pushed
the
feat/angular-devtools
branch
from
20b40f9 to
bdd89ba
Compare
KevinVandy
reviewed
May 19, 2026
| @@ -137,6 +141,12 @@ export function useTable< | |||
| tableOptions: TableOptions<TFeatures, TData>, | |||
| selector?: (state: TableState<TFeatures>) => TSelected, | |||
| ): ReactTable<TFeatures, TData, TSelected> { | |||
| const instanceIdRef = useRef<string | undefined>(undefined) | |||
| const id = useId() | |||
Member
There was a problem hiding this comment.
this technically makes this a React 18+ package now. We could just require an id/key table option in order for devtools to work, and then remove the 2nd arg from useTanStackTableDevtools(table)
Collaborator
Author
There was a problem hiding this comment.
What about removing entirely useId 🤔 ? There is still the generation logic provided next. it was just added as a prefix but probably unnecessary
- const id = useId()
if (!instanceIdRef.current) {
// eslint-disable-next-line @eslint-react/purity
- instanceIdRef.current = `${id}-${Math.random().toString(36).slice(2, 9)}`
+ instanceIdRef.current = Math.random().toString(36).slice(2, 9);
}
Member
There was a problem hiding this comment.
Using math.random for anything in a library usually causes security scanners to freak out, even if it's for something trivial like this.
Collaborator
Author
There was a problem hiding this comment.
switched with crypto.randomUUID (if present) or just id++ for this
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
packages/table-devtools/src/components/StatePanel.tsx (1)
1-1: 💤 Low valueUnused import:
createEffect.
createEffectis imported but not used in this file.🧹 Proposed fix
-import { For, Show, createEffect, createMemo, createSignal } from 'solid-js' +import { For, Show, createMemo, createSignal } from 'solid-js'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/table-devtools/src/components/StatePanel.tsx` at line 1, Remove the unused Solid import createEffect from the import list in StatePanel.tsx; update the import line that currently reads import { For, Show, createEffect, createMemo, createSignal } from 'solid-js' to omit createEffect so only used symbols (For, Show, createMemo, createSignal) remain, ensuring no other references to createEffect exist in the file before committing.packages/table-devtools/src/useTableStore.ts (1)
5-9: 💤 Low valueOutdated documentation.
The comment still claims the hook handles both subscribe APIs (function return for 0.8.x and
{ unsubscribe }for 0.9.x), but the implementation now only handles the{ unsubscribe }object return pattern on line 28.📝 Proposed fix
/** - * Subscribes to a table store and returns a reactive signal. - * Handles both subscribe APIs: function return (store 0.8.x) and - * { unsubscribe } object return (store 0.9.x). + * Subscribes to a table store and returns a reactive signal. */🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/table-devtools/src/useTableStore.ts` around lines 5 - 9, The docblock for useTableStore is outdated: it claims support for both the old function-return subscribe API and the new { unsubscribe } object-return API, but the implementation only handles the object-return pattern; update the comment above useTableStore to state that the hook expects the { unsubscribe } object-return subscription API (remove the mention of the function-return 0.8.x pattern) so the documentation matches the actual behavior of the subscribe handling in useTableStore.packages/table-devtools/src/components/ThreeWayResizableSplit.tsx (1)
23-23: ⚡ Quick winDocument the ESLint suppression rationale.
The suppression is likely justified since event handlers in Solid can read signals without tracking, but adding a brief comment would help future maintainers understand why the rule was disabled.
📝 Suggested documentation
- // eslint-disable-next-line solid/reactivity + // eslint-disable-next-line solid/reactivity -- Event handlers can read signals without tracking (e) => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/table-devtools/src/components/ThreeWayResizableSplit.tsx` at line 23, Add a short rationale comment immediately above the existing eslint suppression for the solid/reactivity rule in ThreeWayResizableSplit (where "// eslint-disable-next-line solid/reactivity" appears) explaining why the rule is disabled (e.g., event handlers intentionally read signals without tracking to avoid creating reactive subscriptions and to prevent unnecessary rerenders). Reference the component name ThreeWayResizableSplit and the specific suppression so maintainers understand this is intentional and safe for the event-handler context.examples/angular/row-selection-signal/src/app/app.config.ts (1)
6-20: ⚡ Quick winProvider array creates nested array in production builds.
The ternary expression
isDevMode() ? provideTanStackDevtools(...) : []produces[[]](an array containing an empty array) when dev mode is false. While Angular may tolerate this, it's structurally incorrect.♻️ Use spread operator to flatten conditional providers
export const appConfig: ApplicationConfig = { providers: [ - isDevMode() - ? provideTanStackDevtools(() => ({ - plugins: [ - { - name: 'TanStack Table', - render: () => - import('`@tanstack/angular-table-devtools`').then((m) => - m.TableDevtoolsPanel(), - ), - }, - ], - })) - : [], + ...(isDevMode() + ? [provideTanStackDevtools(() => ({ + plugins: [ + { + name: 'TanStack Table', + render: () => + import('`@tanstack/angular-table-devtools`').then((m) => + m.TableDevtoolsPanel(), + ), + }, + ], + }))] + : []), ], }Note: This pattern appears across all Angular example app.config.ts files in this PR. Consider applying the fix consistently.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/angular/row-selection-signal/src/app/app.config.ts` around lines 6 - 20, The providers array conditionally returns an array (or empty array) causing a nested array ([[]]) in production; update the providers entry to spread the conditional result so it flattens into the outer array—e.g., replace the current ternary `isDevMode() ? provideTanStackDevtools(...) : []` with `...(isDevMode() ? [provideTanStackDevtools(() => ({ plugins: [...] }))] : [])` (referencing isDevMode and provideTanStackDevtools and the existing devtools plugin block) and apply the same pattern across other app.config.ts examples.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/angular/row-selection-signal/package.json`:
- Line 14: Dependencies for Angular in package.json are inconsistent: update the
downgraded packages (e.g., "`@angular/animations`" and any other Angular packages
currently at "^21.2.12") to the same version used by the rest ("^21.2.13") so
all `@angular/`* entries use a single, consistent version spec; edit the
dependency entries in package.json to replace "^21.2.12" with "^21.2.13" for the
identified Angular package names.
In `@packages/angular-table-devtools/src/injectTanStackTableDevtools.ts`:
- Around line 46-53: The code currently calls
removeTableDevtoolsTarget(registrationId) when enabledValue is falsy or table is
missing but then continues to call upsertTableDevtoolsTarget(...) which
re-registers the target (possibly with an undefined table); update the branch so
that when !enabledValue || !table the function returns early (or otherwise skips
the subsequent upsert) instead of falling through, ensuring
removeTableDevtoolsTarget(registrationId) is the terminal action for that case
and only call upsertTableDevtoolsTarget with { id: registrationId, table, name:
normalizeName(name) } when enabledValue is truthy and table exists.
In `@packages/react-table-devtools/src/useTanStackTableDevtools.ts`:
- Around line 38-54: The effect that registers the devtools target (useEffect)
omits normalizedName from its dependency array, so changes to name won't trigger
upsertTableDevtoolsTarget when instanceId is stable; update the dependency list
for the useEffect that calls upsertTableDevtoolsTarget/removeTableDevtoolsTarget
to include normalizedName alongside enabled, registrationId, and instanceId so
the effect reruns and existingRegistration.name is updated when normalizedName
changes.
- Around line 31-34: The computed instanceId is evaluated unguarded and can
throw when table is undefined, and normalizedName changes are not propagated
because it isn’t in the effect dependencies; inside useTanStackTableDevtools
(function name) move the computation of instanceId into the useEffect that
guards on table (or guard before accessing (table as ...).instanceId) and add
normalizedName to the effect dependency array so instanceId is recomputed when
name changes; ensure the upsertTableDevtoolsTarget call uses the locally
computed instanceId and the current normalizedName so updates propagate
correctly.
In `@packages/react-table/src/useTable.ts`:
- Around line 145-150: The current check "'randomUUID' in globalThis.crypto" can
throw if globalThis.crypto is undefined; update the guard around instanceIdRef
initialization (in useTable) to safely check for crypto and randomUUID — e.g.,
verify globalThis.crypto exists and has a randomUUID function (like typeof
globalThis.crypto?.randomUUID === 'function' or 'randomUUID' in
(globalThis.crypto || {})) before calling it, falling back to the
`table-${++tableId}` path if not available; reference instanceIdRef, useRef,
globalThis.crypto.randomUUID and tableId when making the change.
In `@packages/table-devtools/src/components/Shell.tsx`:
- Around line 52-65: The render-prop parameters passed into Solid's Show are
already resolved values, not accessors—fix the JSX in the Show children to treat
the parameters as values: when destructuring the first Show use the value (e.g.,
tableOptions) as the options prop (options={tableOptions}), and in the inner
Show use the value (e.g., selectedTargetId) as the Select value
(value={selectedTargetId}) and call setSelectedTargetId from the onChange
handler; ensure you still call the accessors tableOptions() and
selectedTargetId() only where you intend to read the signals directly, and
reference Show, tableOptions, selectedTargetId, setSelectedTargetId, Select, and
EMPTY_PANEL_KEY to locate the incorrect props.
---
Nitpick comments:
In `@examples/angular/row-selection-signal/src/app/app.config.ts`:
- Around line 6-20: The providers array conditionally returns an array (or empty
array) causing a nested array ([[]]) in production; update the providers entry
to spread the conditional result so it flattens into the outer array—e.g.,
replace the current ternary `isDevMode() ? provideTanStackDevtools(...) : []`
with `...(isDevMode() ? [provideTanStackDevtools(() => ({ plugins: [...] }))] :
[])` (referencing isDevMode and provideTanStackDevtools and the existing
devtools plugin block) and apply the same pattern across other app.config.ts
examples.
In `@packages/table-devtools/src/components/StatePanel.tsx`:
- Line 1: Remove the unused Solid import createEffect from the import list in
StatePanel.tsx; update the import line that currently reads import { For, Show,
createEffect, createMemo, createSignal } from 'solid-js' to omit createEffect so
only used symbols (For, Show, createMemo, createSignal) remain, ensuring no
other references to createEffect exist in the file before committing.
In `@packages/table-devtools/src/components/ThreeWayResizableSplit.tsx`:
- Line 23: Add a short rationale comment immediately above the existing eslint
suppression for the solid/reactivity rule in ThreeWayResizableSplit (where "//
eslint-disable-next-line solid/reactivity" appears) explaining why the rule is
disabled (e.g., event handlers intentionally read signals without tracking to
avoid creating reactive subscriptions and to prevent unnecessary rerenders).
Reference the component name ThreeWayResizableSplit and the specific suppression
so maintainers understand this is intentional and safe for the event-handler
context.
In `@packages/table-devtools/src/useTableStore.ts`:
- Around line 5-9: The docblock for useTableStore is outdated: it claims support
for both the old function-return subscribe API and the new { unsubscribe }
object-return API, but the implementation only handles the object-return
pattern; update the comment above useTableStore to state that the hook expects
the { unsubscribe } object-return subscription API (remove the mention of the
function-return 0.8.x pattern) so the documentation matches the actual behavior
of the subscribe handling in useTableStore.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7181463f-b364-476b-8721-90df11ac2ddc
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (84)
examples/angular/basic-app-table/package.jsonexamples/angular/basic-app-table/src/app/app.config.tsexamples/angular/basic-app-table/src/app/app.tsexamples/angular/basic-inject-table/package.jsonexamples/angular/basic-inject-table/src/app/app.config.tsexamples/angular/basic-inject-table/src/app/app.tsexamples/angular/column-ordering/package.jsonexamples/angular/column-ordering/src/app/app.config.tsexamples/angular/column-ordering/src/app/app.tsexamples/angular/column-pinning-sticky/package.jsonexamples/angular/column-pinning-sticky/src/app/app.config.tsexamples/angular/column-pinning-sticky/src/app/app.tsexamples/angular/column-pinning/package.jsonexamples/angular/column-pinning/src/app/app.config.tsexamples/angular/column-pinning/src/app/app.tsexamples/angular/column-resizing-performant/package.jsonexamples/angular/column-resizing-performant/src/app/app.config.tsexamples/angular/column-resizing-performant/src/app/app.tsexamples/angular/column-visibility/package.jsonexamples/angular/column-visibility/src/app/app.config.tsexamples/angular/column-visibility/src/app/app.tsexamples/angular/composable-tables/package.jsonexamples/angular/composable-tables/src/app/app.config.tsexamples/angular/composable-tables/src/app/components/products-table/products-table.tsexamples/angular/composable-tables/src/app/components/users-table/users-table.tsexamples/angular/custom-plugin/package.jsonexamples/angular/custom-plugin/src/app/app.config.tsexamples/angular/custom-plugin/src/app/app.tsexamples/angular/editable/package.jsonexamples/angular/editable/src/app/app.config.tsexamples/angular/editable/src/app/app.tsexamples/angular/expanding/package.jsonexamples/angular/expanding/src/app/app.config.tsexamples/angular/expanding/src/app/app.tsexamples/angular/filters/package.jsonexamples/angular/filters/src/app/app.config.tsexamples/angular/filters/src/app/app.tsexamples/angular/grouping/package.jsonexamples/angular/grouping/src/app/app.config.tsexamples/angular/grouping/src/app/app.tsexamples/angular/remote-data/package.jsonexamples/angular/remote-data/src/app/app.config.tsexamples/angular/remote-data/src/app/app.tsexamples/angular/row-dnd/package.jsonexamples/angular/row-dnd/src/app/app.config.tsexamples/angular/row-dnd/src/app/app.tsexamples/angular/row-selection-signal/package.jsonexamples/angular/row-selection-signal/src/app/app.component.tsexamples/angular/row-selection-signal/src/app/app.config.tsexamples/angular/row-selection/package.jsonexamples/angular/row-selection/src/app/app.config.tsexamples/angular/row-selection/src/app/app.tsexamples/angular/signal-input/package.jsonexamples/angular/signal-input/src/app/app.config.tsexamples/angular/signal-input/src/app/person-table/person-table.tsexamples/angular/sub-components/package.jsonexamples/angular/sub-components/src/app/app.config.tsexamples/angular/sub-components/src/app/app.tsexamples/react/composable-tables/package.jsonexamples/react/composable-tables/src/main.tsxpackages/angular-table-devtools/eslint.config.jspackages/angular-table-devtools/package.jsonpackages/angular-table-devtools/src/TableDevtools.tspackages/angular-table-devtools/src/index.tspackages/angular-table-devtools/src/injectTanStackTableDevtools.tspackages/angular-table-devtools/src/plugin.tspackages/angular-table-devtools/src/production.tspackages/angular-table-devtools/tsconfig.jsonpackages/angular-table-devtools/tsdown.config.tspackages/angular-table-devtools/vite.config.tspackages/react-table-devtools/src/useTanStackTableDevtools.tspackages/react-table/src/useTable.tspackages/table-devtools/eslint.config.jspackages/table-devtools/package.jsonpackages/table-devtools/src/TableContextProvider.tsxpackages/table-devtools/src/components/ColumnsPanel.tsxpackages/table-devtools/src/components/FeaturesPanel.tsxpackages/table-devtools/src/components/OptionsPanel.tsxpackages/table-devtools/src/components/RowsPanel.tsxpackages/table-devtools/src/components/Shell.tsxpackages/table-devtools/src/components/StatePanel.tsxpackages/table-devtools/src/components/ThreeWayResizableSplit.tsxpackages/table-devtools/src/tableTarget.tspackages/table-devtools/src/useTableStore.ts
| "private": true, | ||
| "dependencies": { | ||
| "@angular/animations": "^21.2.13", | ||
| "@angular/animations": "^21.2.12", |
Contributor
There was a problem hiding this comment.
Inconsistent Angular package versions.
Two Angular packages are downgraded to 21.2.12 while the rest remain at 21.2.13 (lines 15-19). This version mismatch can cause peer dependency warnings and potential runtime issues.
📦 Align Angular package versions
- "`@angular/animations`": "^21.2.12",
+ "`@angular/animations`": "^21.2.13",- "`@angular/platform-browser-dynamic`": "^21.2.12",
+ "`@angular/platform-browser-dynamic`": "^21.2.13",Also applies to: 20-20
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/angular/row-selection-signal/package.json` at line 14, Dependencies
for Angular in package.json are inconsistent: update the downgraded packages
(e.g., "`@angular/animations`" and any other Angular packages currently at
"^21.2.12") to the same version used by the rest ("^21.2.13") so all `@angular/`*
entries use a single, consistent version spec; edit the dependency entries in
package.json to replace "^21.2.12" with "^21.2.13" for the identified Angular
package names.
Comment on lines
+46
to
+53
| if (!enabledValue || !table) { | ||
| removeTableDevtoolsTarget(registrationId) | ||
| } | ||
| upsertTableDevtoolsTarget({ | ||
| id: registrationId, | ||
| table: table, | ||
| name: normalizeName(name), | ||
| }) |
Contributor
There was a problem hiding this comment.
Stop upserting after the disabled/no-table branch.
Lines 46-53 remove the target but then still re-register it. This makes enabled ineffective and can register an undefined table.
Proposed fix
const { table, name } = options()
const enabledValue = enabled()
if (!enabledValue || !table) {
removeTableDevtoolsTarget(registrationId)
+ return
}
upsertTableDevtoolsTarget({
id: registrationId,
table: table,
name: normalizeName(name),
})📝 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
| if (!enabledValue || !table) { | |
| removeTableDevtoolsTarget(registrationId) | |
| } | |
| upsertTableDevtoolsTarget({ | |
| id: registrationId, | |
| table: table, | |
| name: normalizeName(name), | |
| }) | |
| if (!enabledValue || !table) { | |
| removeTableDevtoolsTarget(registrationId) | |
| return | |
| } | |
| upsertTableDevtoolsTarget({ | |
| id: registrationId, | |
| table: table, | |
| name: normalizeName(name), | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/angular-table-devtools/src/injectTanStackTableDevtools.ts` around
lines 46 - 53, The code currently calls
removeTableDevtoolsTarget(registrationId) when enabledValue is falsy or table is
missing but then continues to call upsertTableDevtoolsTarget(...) which
re-registers the target (possibly with an undefined table); update the branch so
that when !enabledValue || !table the function returns early (or otherwise skips
the subsequent upsert) instead of falling through, ensuring
removeTableDevtoolsTarget(registrationId) is the terminal action for that case
and only call upsertTableDevtoolsTarget with { id: registrationId, table, name:
normalizeName(name) } when enabledValue is truthy and table exists.
Comment on lines
+31
to
+34
| const instanceId = | ||
| // instanceId from react table adapter (if it exists) allows for stable devtools registration even if the table instance changes | ||
| (table as unknown as { instanceId?: string }).instanceId || | ||
| `${registrationId}${normalizedName ? `-${normalizedName}` : ``}` |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/react-table-devtools/src/useTanStackTableDevtools.ts"
if [ ! -f "$FILE" ]; then
echo "File not found: $FILE" >&2
exit 1
fi
# Show the relevant lines with context (keep output small)
nl -ba "$FILE" | sed -n '1,120p' | sed -n '20,90p'Repository: TanStack/table
Length of output: 102
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/react-table-devtools/src/useTanStackTableDevtools.ts"
test -f "$FILE" || { echo "File not found: $FILE" >&2; exit 1; }
# Print only the relevant section (around the reported line range)
# Use cat -n (available) instead of nl.
cat -n "$FILE" | sed -n '1,120p' | sed -n '20,90p'
echo "-----"
# Also extract just the `instanceId` assignment block and the nearest effect deps
rg -n "instanceId\s*=" "$FILE" -n
rg -n "useEffect\(|upsertTableDevtoolsTarget|normalizedName|enabled|registrationId" "$FILE" -nRepository: TanStack/table
Length of output: 2180
Fix potential runtime crash in instanceId and ensure normalizedName updates propagate
- Critical:
instanceIdis computed outside theuseEffectguard; whentableisundefined,(table ...).instanceIdcan throw before theif (!table)check runs. - Minor:
normalizedNameis used inupsertTableDevtoolsTarget({ name: normalizedName }), but it’s not in the effect deps; iftable.instanceIdis present,instanceIdwon’t change whennamechanges, so updates may not propagate.
🐛 Suggested fix
- const instanceId =
- // instanceId from react table adapter (if it exists) allows for stable devtools registration even if the table instance changes
- (table as unknown as { instanceId?: string }).instanceId ||
- `${registrationId}${normalizedName ? `-${normalizedName}` : ``}`
+ const instanceId =
+ // instanceId from react table adapter (if it exists) allows for stable devtools registration even if the table instance changes
+ (table as { instanceId?: string } | undefined)?.instanceId ??
+ `${registrationId}${normalizedName ? `-${normalizedName}` : ``}`📝 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
| const instanceId = | |
| // instanceId from react table adapter (if it exists) allows for stable devtools registration even if the table instance changes | |
| (table as unknown as { instanceId?: string }).instanceId || | |
| `${registrationId}${normalizedName ? `-${normalizedName}` : ``}` | |
| const instanceId = | |
| // instanceId from react table adapter (if it exists) allows for stable devtools registration even if the table instance changes | |
| (table as { instanceId?: string } | undefined)?.instanceId ?? | |
| `${registrationId}${normalizedName ? `-${normalizedName}` : ``}` |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/react-table-devtools/src/useTanStackTableDevtools.ts` around lines
31 - 34, The computed instanceId is evaluated unguarded and can throw when table
is undefined, and normalizedName changes are not propagated because it isn’t in
the effect dependencies; inside useTanStackTableDevtools (function name) move
the computation of instanceId into the useEffect that guards on table (or guard
before accessing (table as ...).instanceId) and add normalizedName to the effect
dependency array so instanceId is recomputed when name changes; ensure the
upsertTableDevtoolsTarget call uses the locally computed instanceId and the
current normalizedName so updates propagate correctly.
Comment on lines
+38
to
+54
| useEffect(() => { | ||
| if (!enabled || !table) { | ||
| removeTableDevtoolsTarget(registrationId) | ||
| removeTableDevtoolsTarget(instanceId) | ||
| return | ||
| } | ||
|
|
||
| upsertTableDevtoolsTarget({ | ||
| id: registrationId, | ||
| id: instanceId, | ||
| table, | ||
| name: normalizeName(name), | ||
| name: normalizedName, | ||
| }) | ||
|
|
||
| return () => { | ||
| removeTableDevtoolsTarget(registrationId) | ||
| removeTableDevtoolsTarget(instanceId) | ||
| } | ||
| }, [enabled, name, registrationId, table]) | ||
| // eslint-disable-next-line @eslint-react/exhaustive-deps,react-hooks/exhaustive-deps | ||
| }, [enabled, registrationId, instanceId]) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/react-table-devtools/src/useTanStackTableDevtools.ts"
echo "=== File header (imports/exports) ==="
sed -n '1,120p' "$FILE" | nl -ba
echo "=== Target effect region (lines ~30-80) ==="
sed -n '1,120p' "$FILE" | nl -ba | sed -n '25,90p'
echo "=== Search for normalizedName definition/usages ==="
rg -n "normalizedName" "$FILE" || true
echo "=== Locate upsertTableDevtoolsTarget/removeTableDevtoolsTarget implementations ==="
rg -n "function upsertTableDevtoolsTarget|upsertTableDevtoolsTarget\\(|removeTableDevtoolsTarget\\(" packages/react-table-devtools -S
# Also search whole repo in case helpers live elsewhere
rg -n "upsertTableDevtoolsTarget\\(|removeTableDevtoolsTarget\\(" -SRepository: TanStack/table
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/react-table-devtools/src/useTanStackTableDevtools.ts"
echo "=== Show lines 1-140 with line numbers (python) ==="
python3 - <<'PY'
path = "packages/react-table-devtools/src/useTanStackTableDevtools.ts"
with open(path, "r", encoding="utf-8") as f:
for i, line in enumerate(f, 1):
if i <= 140:
print(f"{i:4d}: {line.rstrip()}")
PY
echo "=== Search normalizedName definition/usages ==="
rg -n "normalizedName" "$FILE" || true
echo "=== Search for upsert/remove helpers ==="
rg -n "upsertTableDevtoolsTarget\\(|removeTableDevtoolsTarget\\(" -S packages/react-table-devtools || true
rg -n "upsertTableDevtoolsTarget\\(|removeTableDevtoolsTarget\\(" -S . || trueRepository: TanStack/table
Length of output: 6144
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/table-devtools/src/tableTarget.ts"
python3 - <<'PY'
path = "packages/table-devtools/src/tableTarget.ts"
with open(path, "r", encoding="utf-8") as f:
for i, line in enumerate(f, 1):
if i <= 200:
print(f"{i:4d}: {line.rstrip()}")
PY
echo "=== Search for 'name' usage inside tableTarget.ts ==="
rg -n "name\\b" packages/table-devtools/src/tableTarget.tsRepository: TanStack/table
Length of output: 3057
Add normalizedName to the effect deps so devtools target name updates when adapter instanceId is stable.
normalizedName is sent to upsertTableDevtoolsTarget, but it isn’t in the useEffect deps; when the adapter provides a stable instanceId, changing name won’t rerun the effect, so existingRegistration.name won’t be updated.
♻️ Suggested fix
- }, [enabled, registrationId, instanceId])
+ }, [enabled, registrationId, instanceId, normalizedName])📝 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
| useEffect(() => { | |
| if (!enabled || !table) { | |
| removeTableDevtoolsTarget(registrationId) | |
| removeTableDevtoolsTarget(instanceId) | |
| return | |
| } | |
| upsertTableDevtoolsTarget({ | |
| id: registrationId, | |
| id: instanceId, | |
| table, | |
| name: normalizeName(name), | |
| name: normalizedName, | |
| }) | |
| return () => { | |
| removeTableDevtoolsTarget(registrationId) | |
| removeTableDevtoolsTarget(instanceId) | |
| } | |
| }, [enabled, name, registrationId, table]) | |
| // eslint-disable-next-line @eslint-react/exhaustive-deps,react-hooks/exhaustive-deps | |
| }, [enabled, registrationId, instanceId]) | |
| useEffect(() => { | |
| if (!enabled || !table) { | |
| removeTableDevtoolsTarget(instanceId) | |
| return | |
| } | |
| upsertTableDevtoolsTarget({ | |
| id: instanceId, | |
| table, | |
| name: normalizedName, | |
| }) | |
| return () => { | |
| removeTableDevtoolsTarget(instanceId) | |
| } | |
| // eslint-disable-next-line `@eslint-react/exhaustive-deps`,react-hooks/exhaustive-deps | |
| }, [enabled, registrationId, instanceId, normalizedName]) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/react-table-devtools/src/useTanStackTableDevtools.ts` around lines
38 - 54, The effect that registers the devtools target (useEffect) omits
normalizedName from its dependency array, so changes to name won't trigger
upsertTableDevtoolsTarget when instanceId is stable; update the dependency list
for the useEffect that calls upsertTableDevtoolsTarget/removeTableDevtoolsTarget
to include normalizedName alongside enabled, registrationId, and instanceId so
the effect reruns and existingRegistration.name is updated when normalizedName
changes.
Comment on lines
+145
to
+150
| const instanceIdRef = useRef<string | undefined>(undefined) | ||
| if (!instanceIdRef.current) { | ||
| instanceIdRef.current = | ||
| 'randomUUID' in globalThis.crypto | ||
| ? globalThis.crypto.randomUUID() | ||
| : `table-${++tableId}` |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and print the relevant region with line numbers
FILE="packages/react-table/src/useTable.ts"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
echo "---- $FILE (lines ~120-180) ----"
nl -ba "$FILE" | sed -n '120,190p'
# Search for other uses/guards of globalThis.crypto / randomUUID in the codebase
echo "---- Search: globalThis.crypto / randomUUID ----"
rg -n "globalThis\.crypto|randomUUID" packages/react-table/src || true
# Check whether there's any polyfill/assumption about globalThis.crypto earlier in repo
echo "---- Search: randomUUID in repo (small scope) ----"
rg -n "randomUUID" packages/react-table || trueRepository: TanStack/table
Length of output: 184
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/react-table/src/useTable.ts"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
python3 - <<'PY'
from pathlib import Path
path = Path("packages/react-table/src/useTable.ts")
lines = path.read_text(encoding="utf-8").splitlines()
start, end = 120, 190 # 1-based line numbers
for i in range(start, min(end, len(lines)) + 1):
print(f"{i:4d}\t{lines[i-1]}")
PY
echo "---- Search: globalThis.crypto / randomUUID ----"
rg -n "globalThis\.crypto|randomUUID" packages/react-table/src || true
echo "---- Search: crypto polyfills / globalThis assumptions (small scope) ----"
rg -n "globalThis|crypto" packages/react-table || trueRepository: TanStack/table
Length of output: 2655
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- Search entire repo for globalThis.crypto / randomUUID ----"
rg -n "globalThis\.crypto|randomUUID" . || true
echo "---- Search for crypto polyfills or globalThis assignments ----"
rg -n "polyfill|crypto\.randomUUID|require\\('crypto'\\)|globalThis\\.(crypto|msCrypto)" . || true
echo "---- Search for checks like 'crypto' in globalThis ----"
rg -n "'crypto' in globalThis|globalThis\\.crypto\\s*\\?|typeof globalThis\\.crypto" . || trueRepository: TanStack/table
Length of output: 4887
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show root package engines / browserslist (if any)
ls -la package.json >/dev/null 2>&1 && cat package.json || true
echo "----"
for f in packages/*/package.json packages/react-table/package.json; do
if [ -f "$f" ]; then echo "## $f"; cat "$f"; fi
done
echo "---- Search for browserslist / engines ----"
rg -n "\"browserslist\"|browserslist|\"engines\"" -S . || trueRepository: TanStack/table
Length of output: 34086
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- Search for useTable usage in react-table package ----"
rg -n "useTable\\(" packages/react-table || true
echo "---- Search for react-table test setup (jsdom/node) ----"
rg -n "jsdom|testEnvironment|vitest.*environment|environment:" packages/react-table packages/table-core packages/react-table-devtools || true
echo "---- Search README for SSR / crypto assumptions ----"
for f in packages/react-table/README.md packages/react-table-devtools/README.md README.md; do
if [ -f "$f" ]; then
echo "## $f"
rg -n "SSR|server|hydrate|crypto|randomUUID" "$f" || true
fi
doneRepository: TanStack/table
Length of output: 10044
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- packages/react-table/vite.config.ts ----"
cat packages/react-table/vite.config.ts
echo "---- packages/react-table-devtools/vite.config.ts ----"
cat packages/react-table-devtools/vite.config.ts 2>/dev/null || true
echo "---- packages/table-core/vite.config.ts ----"
cat packages/table-core/vite.config.ts 2>/dev/null || true
echo "---- Search for vitest config in react-table ----"
rg -n "vitest|environment\\s*:\\s*'jsdom'|test\\s*:\\s*\\{|jsdom" packages/react-table -S || true
echo "---- Search for SSR notes in useTable docs/comments ----"
rg -n "server-side|SSR|hydrate|globalThis" packages/react-table/src -S || trueRepository: TanStack/table
Length of output: 1668
🏁 Script executed:
node -p "({ crypto: typeof globalThis.crypto, randomUUID: typeof globalThis.crypto?.randomUUID })"Repository: TanStack/table
Length of output: 103
🌐 Web query:
Node 16 globalThis.crypto availability webcrypto experimental global webcrypto
💡 Result:
In Node.js 16, Web Crypto is available via the Web Crypto API implementation, but it is not exposed as globalThis.crypto; you should use require('node:crypto').webcrypto (CommonJS) or import { webcrypto } from 'node:crypto' (ESM) instead.[1][2] What to do in Node 16: 1) CommonJS const { webcrypto } = require('node:crypto'); const { subtle } = webcrypto; 2) ESM import { webcrypto } from 'node:crypto'; const { subtle } = webcrypto; For contrast: current Node.js docs (e.g., newer versions) describe accessing the same Web Crypto API via globalThis.crypto or require('node:crypto').webcrypto.[3][4] Notes: - Node’s Web Crypto API is implemented as the standard Web Crypto API, with globalThis.crypto being an instance of the Crypto singleton in environments where it’s available.[3][4]
Citations:
- 1: https://nodejs.org/docs/latest-v16.x/api/webcrypto.html
- 2: https://developer.mozilla.org/en-US/docs/Web/API/Web_Crypto_API
- 3: http://nodejs.org/dist/latest/docs/api/webcrypto.html
- 4: https://nodejs.org/api/webcrypto.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "globalThis\\.crypto|randomUUID" packages/react-table/src || trueRepository: TanStack/table
Length of output: 221
Guard globalThis.crypto before probing randomUUID.
'randomUUID' in globalThis.crypto throws a TypeError when globalThis.crypto is missing (possible in Node 16 SSR/server runtimes since @tanstack/react-table supports node >=16). Use a guarded check for randomUUID instead.
🐛 Suggested fix
if (!instanceIdRef.current) {
- instanceIdRef.current =
- 'randomUUID' in globalThis.crypto
- ? globalThis.crypto.randomUUID()
- : `table-${++tableId}`
+ const cryptoObj = globalThis.crypto
+ instanceIdRef.current =
+ typeof cryptoObj?.randomUUID === 'function'
+ ? cryptoObj.randomUUID()
+ : `table-${++tableId}`
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/react-table/src/useTable.ts` around lines 145 - 150, The current
check "'randomUUID' in globalThis.crypto" can throw if globalThis.crypto is
undefined; update the guard around instanceIdRef initialization (in useTable) to
safely check for crypto and randomUUID — e.g., verify globalThis.crypto exists
and has a randomUUID function (like typeof globalThis.crypto?.randomUUID ===
'function' or 'randomUUID' in (globalThis.crypto || {})) before calling it,
falling back to the `table-${++tableId}` path if not available; reference
instanceIdRef, useRef, globalThis.crypto.randomUUID and tableId when making the
change.
Comment on lines
+52
to
65
| <Show when={tableOptions().length > 0 && tableOptions()}> | ||
| {(tableOptions) => ( | ||
| <Show when={selectedTargetId() ?? EMPTY_PANEL_KEY}> | ||
| {(selectedTargetId) => ( | ||
| <Select | ||
| label="Table" | ||
| options={tableOptions()} | ||
| value={selectedTargetId()} | ||
| onChange={(value) => setSelectedTargetId(value)} | ||
| /> | ||
| )} | ||
| </Show> | ||
| )} | ||
| </Show> |
Contributor
There was a problem hiding this comment.
Critical: Render prop parameters are values, not functions.
Solid's Show render prop receives the resolved value, not an accessor function. Lines 58-59 incorrectly call the render prop parameters as functions:
- Line 53:
(tableOptions)receives the array returned bytableOptions() - Line 58:
options={tableOptions()}attempts to call that array as a function ❌ - Line 55:
(selectedTargetId)receives the string/value - Line 59:
value={selectedTargetId()}attempts to call that value as a function ❌
This will cause a TypeError: ... is not a function at runtime.
🐛 Proposed fix
<Show when={tableOptions().length > 0 && tableOptions()}>
{(tableOptions) => (
<Show when={selectedTargetId() ?? EMPTY_PANEL_KEY}>
{(selectedTargetId) => (
<Select
label="Table"
- options={tableOptions()}
- value={selectedTargetId()}
+ options={tableOptions}
+ value={selectedTargetId}
onChange={(value) => setSelectedTargetId(value)}
/>
)}
</Show>
)}
</Show>📝 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
| <Show when={tableOptions().length > 0 && tableOptions()}> | |
| {(tableOptions) => ( | |
| <Show when={selectedTargetId() ?? EMPTY_PANEL_KEY}> | |
| {(selectedTargetId) => ( | |
| <Select | |
| label="Table" | |
| options={tableOptions()} | |
| value={selectedTargetId()} | |
| onChange={(value) => setSelectedTargetId(value)} | |
| /> | |
| )} | |
| </Show> | |
| )} | |
| </Show> | |
| <Show when={tableOptions().length > 0 && tableOptions()}> | |
| {(tableOptions) => ( | |
| <Show when={selectedTargetId() ?? EMPTY_PANEL_KEY}> | |
| {(selectedTargetId) => ( | |
| <Select | |
| label="Table" | |
| options={tableOptions} | |
| value={selectedTargetId} | |
| onChange={(value) => setSelectedTargetId(value)} | |
| /> | |
| )} | |
| </Show> | |
| )} | |
| </Show> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/table-devtools/src/components/Shell.tsx` around lines 52 - 65, The
render-prop parameters passed into Solid's Show are already resolved values, not
accessors—fix the JSX in the Show children to treat the parameters as values:
when destructuring the first Show use the value (e.g., tableOptions) as the
options prop (options={tableOptions}), and in the inner Show use the value
(e.g., selectedTargetId) as the Select value (value={selectedTargetId}) and call
setSelectedTargetId from the onChange handler; ensure you still call the
accessors tableOptions() and selectedTargetId() only where you intend to read
the signals directly, and reference Show, tableOptions, selectedTargetId,
setSelectedTargetId, Select, and EMPTY_PANEL_KEY to locate the incorrect props.
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
2 participants
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.
TanStack Devtools Angular package is not published yet in npm
Summary by CodeRabbit