fix: table internal and tests, lower intantiations more#6322
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughSeveral core ChangesTypeScript Type System Refactor and API Fix
GitHub Issue Workflow Skill Document
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
|
View your CI Pipeline Execution ↗ for commit a700803
☁️ Nx Cloud last updated this comment at |
![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: 2
🤖 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
`@packages/table-core/src/features/column-pinning/columnPinningFeature.utils.ts`:
- Line 367: There is a type handling inconsistency across feature utility files
following the Column_Internal type narrowing refactor. In
columnPinningFeature.utils.ts, the explicit type cast was removed and the code
uses the public Column type instead, but in columnFilteringFeature.utils.ts and
globalFilteringFeature.utils.ts, as any casts are still being used. Update
columnFilteringFeature.utils.ts and globalFilteringFeature.utils.ts to follow
the same pattern as columnPinningFeature.utils.ts: remove the as any casts
applied to orderedLeafColumns or similar column array variables, and instead
rely on the public Column type (Column<TFeatures, TData, unknown>) to preserve
the full type contract after the Column_Internal narrowing refactor, ensuring
consistent type handling across all three files.
In `@packages/table-core/src/types/Column.ts`:
- Around line 45-51: The type narrowing in Column_Internal omits the
ExtractFeatureMapTypes intersection present in the broader Column type, causing
type mismatches. Update the callback function signatures that currently expect
Column to instead accept Column_Internal: In columnFilteringFeature.types.ts
(where filterFn.autoRemove is defined around line 83), change the parameter type
to Column_Internal, and in globalFilteringFeature.types.ts (where
getColumnCanGlobalFilter is defined around line 52), change the parameter type
to Column_Internal. Then remove the as any casts at the call sites in
columnFilteringFeature.utils.ts (line 326) and globalFilteringFeature.utils.ts
(line 29) respectively, since the types will now match correctly.
🪄 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: 4ed4f2ef-a4ec-4e1c-abf4-7e4755e16102
📒 Files selected for processing (20)
examples/react/kitchen-sink/src/main.tsxpackages/table-core/src/core/columns/coreColumnsFeature.utils.tspackages/table-core/src/core/headers/buildHeaderGroups.tspackages/table-core/src/features/column-filtering/columnFilteringFeature.utils.tspackages/table-core/src/features/column-pinning/columnPinningFeature.utils.tspackages/table-core/src/features/column-resizing/columnResizingFeature.types.tspackages/table-core/src/features/global-filtering/globalFilteringFeature.utils.tspackages/table-core/src/helpers/columnHelper.tspackages/table-core/src/types/Column.tspackages/table-core/src/types/HeaderGroup.tspackages/table-core/src/types/RowModel.tspackages/table-core/src/types/RowModelFns.tspackages/table-core/src/types/Table.tspackages/table-core/src/types/TableState.tspackages/table-core/tests/helpers/rowPinningHelpers.tspackages/table-core/tests/implementation/features/row-pinning/rowPinningFeature.test.tspackages/table-core/tests/unit/core/tableAtoms.test.tspackages/table-core/tests/unit/features/column-pinning/columnPinningFeature.utils.test.tspackages/table-core/tests/unit/features/row-pinning/rowPinningFeature.utils.test.tsskills/github-issue-repro-and-fix/SKILL.md
| string, | ||
| Column_Internal<TFeatures, TData, unknown> | ||
| > | ||
| const leafColumnsById = table.getAllLeafColumnsById() |
Contributor
There was a problem hiding this comment.
Type cast removal at line 367 reflects Column_Internal narrowing.
The explicit cast previously applied to leafColumnsById has been removed, and at lines 371 and 405, the orderedLeafColumns arrays now use the public Column<TFeatures, TData, unknown> type instead of Column_Internal<...>.
This change is consistent with the Column_Internal type narrowing in Column.ts (lines 45-51): since Column_Internal no longer includes feature-specific types, this code switched to the public Column type to preserve the full type contract.
However, this creates an inconsistency with columnFilteringFeature.utils.ts and globalFilteringFeature.utils.ts, which use as any casts instead of switching to Column. The mixed approaches suggest the type refactor may not have a consistent resolution strategy.
🤖 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-core/src/features/column-pinning/columnPinningFeature.utils.ts`
at line 367, There is a type handling inconsistency across feature utility files
following the Column_Internal type narrowing refactor. In
columnPinningFeature.utils.ts, the explicit type cast was removed and the code
uses the public Column type instead, but in columnFilteringFeature.utils.ts and
globalFilteringFeature.utils.ts, as any casts are still being used. Update
columnFilteringFeature.utils.ts and globalFilteringFeature.utils.ts to follow
the same pattern as columnPinningFeature.utils.ts: remove the as any casts
applied to orderedLeafColumns or similar column array variables, and instead
rely on the public Column type (Column<TFeatures, TData, unknown>) to preserve
the full type contract after the Column_Internal narrowing refactor, ensuring
consistent type handling across all three files.
Comment on lines
+45
to
51
| export interface Column_Internal< | ||
| in out TFeatures extends TableFeatures, | ||
| in out TData extends RowData, | ||
| TValue = unknown, | ||
| > = Column<TFeatures, TData, TValue> & { | ||
| > extends Omit<Column_Core<TFeatures, TData, TValue>, 'columnDef'> { | ||
| columnDef: ColumnDefBase_All<TFeatures, TData, TValue> | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Identify all Column_Internal usages and `as any` casts related to column types
echo "=== All Column_Internal usages ==="
rg -n 'Column_Internal<' --type=ts -C 1
echo ""
echo "=== Column type casts to any (potential type gaps) ==="
rg -n 'column.*as any' --type=ts -C 2
echo ""
echo "=== FilterFn and callback signatures expecting column parameters ==="
rg -nP 'autoRemove.*\(|getColumnCanGlobalFilter.*\(' --type=ts -C 2Repository: TanStack/table
Length of output: 50370
🏁 Script executed:
# Read the Column_Internal definition and related types
cat -n packages/table-core/src/types/Column.ts | sed -n '40,65p'Repository: TanStack/table
Length of output: 561
🏁 Script executed:
# Look at FilterFn type definition and autoRemove signature
rg -n 'interface FilterFn|type FilterFn' --type=ts -A 10Repository: TanStack/table
Length of output: 3792
🏁 Script executed:
# Find getColumnCanGlobalFilter callback signature
rg -n 'getColumnCanGlobalFilter' packages/table-core/src --type=ts -B 2 -A 2Repository: TanStack/table
Length of output: 2502
🏁 Script executed:
# Find ColumnFilterAutoRemoveTestFn definition
rg -n 'ColumnFilterAutoRemoveTestFn' packages/table-core/src --type=ts -B 1 -A 5Repository: TanStack/table
Length of output: 1677
🏁 Script executed:
# Get the full getColumnCanGlobalFilter signature
rg -n 'getColumnCanGlobalFilter' packages/table-core/src/features/global-filtering/globalFilteringFeature.types.ts -A 8Repository: TanStack/table
Length of output: 346
🏁 Script executed:
# Check if Column_Internal ever intersected with Column (check git or find evidence of expected type)
cat -n packages/table-core/src/types/Column.ts | sed -n '39,75p'Repository: TanStack/table
Length of output: 603
🏁 Script executed:
# Verify these are the only two production code as any casts related to this issue
# Filter out test files and examples
rg -n 'column.*as any' packages/table-core/src --type=ts | grep -v test | grep -v exampleRepository: TanStack/table
Length of output: 359
🏁 Script executed:
# Check context around the function that uses these callbacks
cat -n packages/table-core/src/features/column-filtering/columnFilteringFeature.utils.ts | sed -n '310,335p'Repository: TanStack/table
Length of output: 773
Type narrowing in Column_Internal causes downstream type-safety gaps.
The type definition of Column_Internal extends only Omit<Column_Core<...>, 'columnDef'>, omitting the ExtractFeatureMapTypes intersection present in the broader Column type. This forces two callback functions to suppress type safety with as any casts:
packages/table-core/src/features/column-filtering/columnFilteringFeature.utils.ts#L326: Castscolumntoanywhen passing tofilterFn.autoRemove, which expectsColumn<TFeatures, TData, TValue>(line 83 of columnFilteringFeature.types.ts).packages/table-core/src/features/global-filtering/globalFilteringFeature.utils.ts#L29: Castscolumntoanywhen passing togetColumnCanGlobalFilter, which expectsColumn<TFeatures, TData, TValue>(line 52 of globalFilteringFeature.types.ts).
Adjust the callback signatures to accept Column_Internal explicitly rather than relying on any to suppress the type mismatch.
📍 Affects 3 files
packages/table-core/src/types/Column.ts#L45-L51(this comment)packages/table-core/src/features/column-filtering/columnFilteringFeature.utils.ts#L326-L326packages/table-core/src/features/global-filtering/globalFilteringFeature.utils.ts#L29-L29
🤖 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-core/src/types/Column.ts` around lines 45 - 51, The type
narrowing in Column_Internal omits the ExtractFeatureMapTypes intersection
present in the broader Column type, causing type mismatches. Update the callback
function signatures that currently expect Column to instead accept
Column_Internal: In columnFilteringFeature.types.ts (where filterFn.autoRemove
is defined around line 83), change the parameter type to Column_Internal, and in
globalFilteringFeature.types.ts (where getColumnCanGlobalFilter is defined
around line 52), change the parameter type to Column_Internal. Then remove the
as any casts at the call sites in columnFilteringFeature.utils.ts (line 326) and
globalFilteringFeature.utils.ts (line 29) respectively, since the types will now
match correctly.
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.Summary by CodeRabbit
Bug Fixes
Documentation