fix(grid): fix columnIndex error #3890
Conversation
WalkthroughIndexing and column-list construction were adjusted: getColumnIndex now reads the per-entry Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/renderless/src/grid/utils/common.ts(2 hunks)packages/vue/src/grid/src/table/src/methods.ts(2 hunks)packages/vue/src/grid/src/table/src/table.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/vue/src/grid/src/table/src/table.ts (1)
packages/vue-common/src/index.ts (1)
hooks(477-477)
packages/vue/src/grid/src/table/src/methods.ts (1)
packages/renderless/src/grid/utils/common.ts (1)
getColumnList(45-78)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (6)
packages/vue/src/grid/src/table/src/methods.ts (1)
998-1005: LGTM!The initialization of
markColumnIndexto 0 before callinggetColumnListensures fresh sequential indexing for each column rebuild. Passingthisas the first parameter correctly provides the table instance context needed by the updatedgetColumnListsignature.packages/vue/src/grid/src/table/src/table.ts (2)
810-810: LGTM!The reactive
markColumnIndexcounter is correctly initialized and will be used bygetColumnListto assign sequential indexes to leaf columns during traversal.
845-846: LGTM!Exposing
markColumnIndexin the setup return allows methods (likewatchColumn) to access and reset this counter as needed for column index tracking.packages/renderless/src/grid/utils/common.ts (3)
65-72: LGTM!The new logic correctly assigns sequential
columnIndexvalues only to leaf columns (columns without children) using the$table.markColumnIndexcounter. Non-leaf parent/group columns receivenull, which is appropriate for hierarchical column structures.
74-74: LGTM!The recursive call correctly passes
$tableas the first parameter, ensuring themarkColumnIndexcounter is shared across all levels of the column hierarchy.
45-45: All callers ofgetColumnListhave been correctly updated.The function signature change to add
$tableas the first parameter has been properly reflected in both call sites: the external call at line 1005 inpackages/vue/src/grid/src/table/src/methods.tsand the recursive call at line 74 inpackages/renderless/src/grid/utils/common.ts.
| getColumnIndex(column) { | ||
| const { fullColumnMap } = this | ||
|
|
||
| return fullColumnMap.has(column) ? fullColumnMap.get(column).index : -1 | ||
| return fullColumnMap.has(column) ? fullColumnMap.get(column).columnIndex : -1 | ||
| }, |
There was a problem hiding this comment.
Verify handling of non-leaf column indexes.
The new implementation returns columnIndex from the cached map entry, which will be null for non-leaf (parent/group) columns. This differs from the fallback -1 for columns not found in the map, potentially leading to inconsistent return types (null vs -1).
Consider normalizing the return value:
getColumnIndex(column) {
const { fullColumnMap } = this
- return fullColumnMap.has(column) ? fullColumnMap.get(column).columnIndex : -1
+ return fullColumnMap.has(column) ? (fullColumnMap.get(column).columnIndex ?? -1) : -1
},🤖 Prompt for AI Agents
In packages/vue/src/grid/src/table/src/methods.ts around lines 579 to 583, the
method returns fullColumnMap.get(column).columnIndex which can be null for
non-leaf (group) columns, causing inconsistent return types (null vs -1); change
the logic to normalize the result to a number by returning the stored
columnIndex when it is a valid number and returning -1 when the map has no entry
or when columnIndex is null/undefined so the method always returns -1 for
non-leaf or missing columns.
PR
修复columnIndex 错误问题
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.