refactor: optimize table performance and refactor the table #3514
Conversation
WalkthroughThis update delivers a major refactor and modernization of the grid/table component system, transitioning from Vue 2 options API to Vue 3 composition API, introducing new composable utilities, and overhauling the rendering and data management logic. It adds new features for column/row pooling, header/footer management, cell event handling, and data structuring, while removing legacy header/footer components and utilities. Numerous bug fixes, test improvements, and style adjustments are also included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GridComponent
participant Composables
participant TableData
participant DOM
User->>GridComponent: Interacts (sort/filter/edit/scroll)
GridComponent->>Composables: Calls usePool/useCellEvent/useData/etc.
Composables->>TableData: Pool, structure, or update data
TableData-->>Composables: Returns processed data/graph
Composables-->>GridComponent: Provides reactive refs for rendering
GridComponent->>DOM: Renders via JSX/TSX
User->>DOM: Sees updated grid, interacts again
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
examples/sites/demos/pc/app/grid/custom/column-fixed.spec.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. examples/sites/demos/pc/app/grid/custom/page-size.spec.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. examples/sites/demos/pc/app/grid/data-source/request-service.spec.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 29
🔭 Outside diff range comments (3)
examples/sites/demos/pc/app/grid/event/toolbar-button-click-event.spec.js (1)
6-8: First row-count assertion should also use:visiblefor consistencyAfter the insert you count all rows (
.tiny-grid-body__row) but after delete you count visible rows only.
If any row is hidden (e.g. virtual scroll, filtering) the first assertion can become flaky.- await expect(page.locator('.tiny-grid-body__row')).toHaveCount(7) + await expect(page.locator('.tiny-grid-body__row:visible')).toHaveCount(7)This keeps both checks measuring the same thing and avoids false positives.
Also applies to: 18-18
packages/vue/src/grid/src/checkbox/src/methods.ts (1)
131-135: Encode row key before lookup or selection restore silently fails
fullDataRowIdDatauses URL-encoded keys (see usage inhandleSelectionDefChecked), but here the raw key is used, so existing selections are not restored for rows whose id contains special characters.- let rowCache = fullDataRowIdData[`${get(row, rowkey)}`] + const encoded = encodeURIComponent(get(row, rowkey)) + let rowCache = fullDataRowIdData[encoded]packages/vue/src/grid/src/keyboard/src/methods.ts (1)
62-88: Null-safety gap inwriteClipboardText
rowNodes.forEachis called unguarded but the caller can passundefined(e.g. on customwriteMethod).
Add a default empty array to prevent runtime errors:-const writeClipboardText = ({ $table, columns, rows, rowNodes }) => { +const writeClipboardText = ({ $table, columns, rows, rowNodes = [] }) => {
🧹 Nitpick comments (41)
examples/sites/demos/pc/app/grid/expand/set-row-expansion.spec.js (1)
5-9: Viewport hard-coded per test – consider project-wide defaultSetting an explicit
setViewportSizeeliminates flaky virtual-scroll behaviour – good call.
If multiple specs need the same viewport, you can avoid duplication by adding:// playwright.config.ts test.use({ viewport: { width: 1400, height: 2500 } })or a dedicated
fixturefile.examples/sites/demos/pc/app/grid/custom/page-size.spec.js (1)
7-8: Align both assertions to the same visibility ruleThe first count still includes hidden (virtualised) rows, while the second uses
:visible. Mixing rules can introduce flaky failures if virtual scrolling is enabled.- await expect(demo.locator('.tiny-grid-body__row')).toHaveCount(10) + await expect(demo.locator('.tiny-grid-body__row:visible')).toHaveCount(10)examples/sites/demos/pc/app/grid/mouse-keyboard/keyboard-navigation.spec.js (1)
7-10: Set viewport before navigation for determinismChanging the viewport after
page.gotocan trigger a second re-layout and occasionally shift focus/scroll positions, leading to intermittent test failures. Setting it first makes the page load with the desired size.- await page.goto('grid-mouse-keyboard#mouse-keyboard-keyboard-navigation') - await page.setViewportSize({ + await page.setViewportSize({ width: 1400, height: 2500 }) + await page.goto('grid-mouse-keyboard#mouse-keyboard-keyboard-navigation')examples/sites/demos/pc/app/grid/renderer/inner-renderer.spec.js (1)
6-9: Apply the same preload-viewport patternFor consistency with other specs and to avoid double re-layout, consider moving
setViewportSizeabovegoto, mirroring the suggestion in the keyboard-navigation test.examples/sites/demos/pc/app/grid/size/max-min-grid-height.spec.js (1)
6-7: Consider tolerance when asserting computed CSS valuesHard-coding exact pixel values can break when themes or DPI scaling change. A more resilient approach:
await expect( page.locator('.tiny-grid__body-wrapper').nth(0).evaluate( (el) => parseFloat(getComputedStyle(el).maxHeight) ) ).resolves.toBeGreaterThanOrEqual(200)Same applies to the
min-heightassertion.examples/sites/demos/pc/app/grid/large-data/full-data-loading.spec.js (2)
6-9: Viewport hard-coding improves determinism but limits flexibility.Locking the viewport guarantees consistent scroll geometry across CI agents — good move.
Consider extracting the dimensions to a helper or constant so multiple specs stay in sync and future tweaks are one-liner.
19-20: Magic number in assertion may be brittle.Expecting cell
'129'after a 5 000 px scroll ties the test to the current data volume & row height.
If those change, the test fails even though functionality is fine. Prefer asserting on relative behaviour (e.g. row index visibility) or computing the expected value dynamically.examples/sites/demos/pc/app/grid/webdoc/grid-empty.js (1)
29-33: Update English description for parity with Chinese note
zh-CNcopy was updated to mention the new default behaviour from v3.25.0, but theen-UStext was left unchanged.
To avoid confusing English-speaking readers, please add the same version note to the English string.packages/theme/src/grid/body.less (1)
24-33:no-datamodifier added only to right-fixed body in base theme – consider consistencyIn the SaaS theme the same
.no-datarules are applied to all three body wrappers.
For the base theme you added it only to__fixed-right-body-wrapper, which may leave gaps in the left / main body when no-data is shown..@{grid-prefix-cls}__body-wrapper, .@{grid-prefix-cls}__fixed-left-body-wrapper, .@{grid-prefix-cls}__fixed-right-body-wrapper { overflow-y: auto; overflow-x: auto; + + &.no-data { + ... + } }Unifying the scope keeps behaviour identical across themes and prevents future divergence.
examples/sites/demos/pc/app/grid/custom/column-fixed.spec.js (1)
6-6: Use a more descriptive locator variable name to avoid ambiguity.
customanddemoare both locators to grid containers. Consider renaming one of them (e.g.gridandsettingsPanel) to make it obvious which node each variable targets and reduce cognitive load when reading the test.packages/vue-icon-saas/index.ts (1)
93-96: Icon is properly wired-in – remember to update changelog/demo docsThe import, named export, and default-export entries for
IconDelegatedProcessinglook consistent with the existing pattern.
Just make sure the public docs / icon list are refreshed so consumers know the new token exists.Also applies to: 861-863, 1746-1747
packages/vue/src/grid/src/mobile-first/index.vue (1)
48-49: Explicit prop typing could be strongerMoving away from
$propsclarifies the contract, butArray / Objectare very loose.
Consider tightening with generics orPropType<>to gain type-safety & runtime validation:- props: { - tableData: Array, - cardConfig: Object, - listConfig: Object, - ganttConfig: Object, - customConfig: Object - }, + props: { + tableData: { type: Array as PropType<YourRowType[]>, default: () => [] }, + cardConfig: { type: Object as PropType<CardConfig>, default: () => ({}) }, + listConfig: { type: Object as PropType<ListConfig>, default: () => ({}) }, + ganttConfig: { type: Object as PropType<GanttConfig>, default: () => ({}) }, + customConfig: { type: Object as PropType<CustomConfig>, default: () => ({}) } + },Also applies to: 65-71
examples/sites/demos/pc/app/grid/editor/custom-edit.spec.js (1)
6-12: Prefer semantic Playwright selectors for resilienceHard-coding the DOM path (
.tiny-grid-body__row … td … .tiny-input__inner) makes the test brittle against future refactors of the grid markup.
Playwright’s role- or text-based APIs are more stable and self-documenting, e.g.await expect( demo.getByRole('cell', { row: 1, column: 2 }).locator('input') ).toBeVisible()Consider switching to those selectors so test maintenance cost stays low when table internals change.
packages/vue/src/grid/src/tools/index.ts (1)
48-55: Avoid repeatedtoRawcalls
hooks.toRaw(row)is invoked twice per path. Cache it locally for micro-perf and readability.- if (cacheFormat) { - rest = $table.fullDataRowMap.get(hooks.toRaw(row)) + if (cacheFormat) { + const rawRow = hooks.toRaw(row) + rest = $table.fullDataRowMap.get(rawRow)Apply the same optimisation where the map is accessed again.
packages/vue/src/grid/src/tooltip/src/methods.ts (1)
38-43: Tooltip now relies solely onhoverText– verify it is always setThe previous fallback logic for header cells / icon clicks was removed.
Ifthis.hoverTextisn’t updated on certain mouse events the function returns early and no tooltip is shown, potentially regressing UX for header sort/filter icons.Consider keeping minimal fallback:
let cell = this.hoverText || event.target.closest('.tiny-grid-cell')to preserve behaviour.
packages/vue/src/grid/src/checkbox/src/methods.ts (1)
217-226: Minor: protect against missing theme entryIf a theme/size combination is absent in
GlobalConfig.rowHeightthe expression returnsundefined || 40, which is safe but makes debugging harder. Consider a clearer fallback:-const rowHeight = GlobalConfig.rowHeight[tinyTheme]?.[vSize || 'default'] || 40 +const rowHeight = + (GlobalConfig.rowHeight?.[tinyTheme] && + GlobalConfig.rowHeight[tinyTheme][vSize || 'default']) ?? + 40packages/vue/src/grid/src/body/src/usePool.ts (1)
3-5:differencehelper is O(n²); consider aSetfor large dataFor large tables the current
findIndexapproach degrades quickly.
Building aSetfrom the new ids yields linear complexity:-const difference = (arr, other) => arr.filter((i) => other.findIndex((j) => i.id === j.id) === -1) +const difference = (prev, next) => { + const ids = new Set(next.map((i) => i.id)) + return prev.filter((i) => !ids.has(i.id)) +}packages/vue/src/grid/src/tree/src/methods.ts (1)
51-56: Ternary chain returns meaningless''
expandAll ? … : rowids ? … : ''yields an empty string in the “no-expand”
case, which is confusing and unused.-expandAll ? doExpandAll() : rowids ? doExpandRows() : '' +if (expandAll) { + doExpandAll() +} else if (rowids) { + doExpandRows() +}packages/vue/src/grid/src/adapter/src/renderer.ts (1)
68-82: Micro-task cascade on every keystroke may hurt performanceWrapping every input/change in
Promise.resolve().then(...)schedules a micro-task
per keypress, which can flood the queue on fast typing.
If immediate reactivity is not required, debounce or batch the update:- Promise.resolve().then(() => { - $table.updateStatus(params, cellValue, renderOpts) - }) + queueMicrotask(() => + $table.updateStatus(params, cellValue, renderOpts) + )Or defer until blur/commit phase for heavy grids.
packages/vue/src/grid/src/config.ts (1)
171-180: New height maps – make them theme-agnosticThe hard-coded
tiny/saaskeys work, but mean a 3rd theme requires code changes.
Export the map and allow themes to register their own overrides instead.packages/vue/src/grid/src/edit/src/methods.ts (1)
609-612: Debounce wrapper’s return value is discarded
debounce(20, selectMethod)returns a wrapped function whose own return
value bubbles the debounced execution, not the original promise.
Call sites expect only a next-tick promise, so this is probably fine,
but add a comment to avoid future misuse.packages/vue/src/grid/src/composable/useDrag/index.ts (1)
133-138: Throttle logic tightly couples to hard-coded 2 s
timeis fixed at 2000 ms, but different apps may require shorter/longer delays. Consider exposing it (props.dropConfig.saveDelay?) or at least a named constant exported from config for easy tuning.packages/vue/src/grid/src/table/src/utils/updateStyle.ts (1)
35-40: Avoid parsing non-percentage heights twice
parseInt(height)runs before we even know the value is a percentage, so'100px'enters the percentage branch and wastes CPU. Re-order the test:-if (height === 'auto') { +if (height === 'auto') { ... -} else { - scaleToPx = Math.floor((parseInt(height) / 100) * parentHeight) - customHeight = isScale(height) ? scaleToPx : toNumber(height) +} else if (isScale(height)) { + scaleToPx = Math.floor((parseInt(height) / 100) * parentHeight) + customHeight = scaleToPx +} else { + customHeight = toNumber(height) }packages/vue/src/grid/src/composable/useCellSpan.ts (1)
76-104: Heavy watchers could be debouncedBoth span watchers recalculate the entire matrix on every minor prop change. For large datasets this is expensive. Wrapping the inner body with
hooks.nextTick+debounce(e.g. 16 ms) would smooth bursts from column-resize or rapid scrolling.packages/vue/src/grid/src/composable/useHeader.ts (1)
97-105: Initial header isn’t built untilcollectColumnmutatesThe first
watchlacks{ immediate: true }; on fresh mountheader.valueisundefined, so header height/rows remain blank until the columns array changes.-hooks.watch( - () => props.collectColumn, - () => { +hooks.watch( + () => props.collectColumn, + () => { ... - } + }, + { immediate: true } )packages/vue/src/grid/src/composable/useRowGroup.ts (1)
74-81: Avoiddeleteon reactive objects
delete column._rowGroupColspan&delete column._stickyClasshamper reactivity and hit V8’s optimisation.
Prefer setting them toundefined(ornull) instead.- delete column._rowGroupColspan - delete column._stickyClass + column._rowGroupColspan = undefined + column._stickyClass = undefinedpackages/renderless/src/grid/utils/common.ts (1)
74-95:repairFixedmutates every descendant – verify side-effects
repairFixedcopies the ancestor’sfixedproperty to all descendants, overwriting any explicit child configuration.
If a nested column intentionally setsfixed = null/undefinedto opt-out, that information is lost.Consider:
-if (fixed) { - subtree.forEach((c) => (c.fixed = fixed)) +if (fixed) { + subtree.forEach((c) => { + if (c.fixed === undefined) c.fixed = fixed // honour explicit overrides + }) }Please double-check desired behaviour before merging.
packages/theme-saas/src/grid/table.less (2)
1360-1386: Sticky header columns inadvertently inherit body stylesThe new block targets
.tiny-grid-header__columnunder.tiny-grid__body, making header cells sticky inside the body section.
When virtual scrolling is enabled this may cause duplicate header rows, unexpected z-index stacking, and extra reflow costs.Verify that the selector should indeed be
.tiny-grid__body .tiny-grid-header__column; otherwise limit it to the header wrapper.
871-875:empty-blocknow usesposition: sticky; check accessibility overlayChanging the “no-data” placeholder to
stickymeans it can overlap footer or virtual scroll placeholders.
Ensure tabindex / aria attributes are still reachable or switch toposition: absoluteif overlap is unintended.packages/vue/src/grid/src/composable/useData.ts (1)
118-126: Potential memory leak –_tileInfo/_graphInfonever clearedThe composable stores heavy structures on
$tablebut provides no teardown.
If the table instance is cached (keep-alive) or recreated in the same Vue component, stale references remain.Add a
onBeforeUnmounthook:hooks.onBeforeUnmount(() => { delete $table._tileInfo delete $table._graphInfo })packages/vue/src/grid/src/grid/grid.ts (1)
162-164: Unused reactive state
isInitialLoadingand_delayActivateAnchorare added todata()but are never read afterwards. Dead state bloats the instance and makes intent unclear – please remove or wire them up.packages/vue/src/grid/src/keyboard/src/methods.ts (1)
280-289: Mis-named local variable
mouseConfig.excludesis assigned toexcludeClo, but later the code still callsexcludeClo.includes.
The typo is harmless, yet confusing – rename toexcludesfor clarity.packages/vue/src/grid/src/table/src/strategy.ts (1)
10-11:differenceruns in O(n²)`With many columns this becomes quadratic. Build a lookup set for
otherfirst to make it O(n):const otherSet = new Set(other.map(c=>c.id)) return arr.filter(c=>!otherSet.has(c.id))packages/renderless/src/grid/utils/dom.ts (1)
39-46:updateCellTitleignores caller’seventThe refactor now prefers the explicit
td, making theeventparameter redundant.
Either drop theeventargument or fall back consistently; current mixed logic is confusing.packages/vue/src/grid/src/body/src/body.tsx (3)
436-456: Avoid using delete operator for better performance.The
deleteoperator can cause performance degradation by changing object shape and potentially deoptimizing code.- // 防止数据多次刷新导致key回归rowid - _vm.$nextTick(() => { - delete row._isDraging - }) + // 防止数据多次刷新导致key回归rowid + _vm.$nextTick(() => { + row._isDraging = undefined + })
837-837: Remove unnecessary aliasing ofthis.Arrow functions inherit
thisfrom their enclosing scope, making this alias unnecessary.- const _vm = thisAnd update the usage in line 871:
- ? $slots.empty.call(_vm, { $table }, h) + ? $slots.empty.call(this, { $table }, h)
772-785: Document the reason for the 50ms delay.The 50ms timeout for initializing row/column sortable functionality uses a magic number without explanation.
- // 初始化行列拖拽 + // 初始化行列拖拽 (延迟确保DOM完全渲染) setTimeout(() => { const { dropConfig } = $table if (dropConfig) { const { plugin, row = true, column = true, scheme } = dropConfig plugin && row && (vm.rowSortable = $table.rowDrop(body.value)) if (scheme !== 'v2') { plugin && column && (vm.columnSortable = $table.columnDrop(body.value)) } } }, 50)packages/vue/src/grid/src/table/src/table.ts (2)
322-415: Consider extracting state management into dedicated composables.The
data()function contains a large amount of state (90+ lines) which could make the component harder to maintain. Consider grouping related state into composables for better organization and reusability.For example, you could create:
useEditStatefor edit-related state (editStore, validStore, validatedMap)useScrollStatefor scroll-related state (scrollLoadStore, overflowX, overflowY, etc.)useSelectionStatefor selection-related state (selection, selectRow, isAllSelected, etc.)useMenuStatefor context menu state (ctxMenuStore)This would make the component more modular and easier to test.
829-940: Consider breaking down the complex render function.The render function is quite complex with many conditionals and nested elements. While functional, it could benefit from being broken down into smaller, more focused render functions.
Consider extracting parts like:
renderHiddenColumns()for the hidden containerrenderMainContent()for the grid body and bordersrenderOverlays()for loading, filters, menus, and tooltipsrenderToolbar()for the select toolbarThis would improve readability and make the component easier to maintain.
packages/vue/src/grid/src/table/src/methods.ts (2)
1059-1097: Hard-coded scrollbar size & double reflow
scrollbarSizeis fixed to10, but on Windows / macOS overlay scrollbars vary (0-17 px). Mis-estimation breaks the “has scroll-X/scroll-Y” logic.Use
getBoundingClientRectdiffing or a util (getScrollbarWidth()) instead of a magic number.Additionally
autoCellWidth()is called twice inrecalculate()(before & aftercomputeScrollLoad()), causing two full-layout passes. Consider calling once after scroll metrics are known.
2031-2056:attemptRestoreScroll– flag never reset
this.restoreScollFlag = true(note: typo “Scoll”) is set but never turned off, so subsequent scroll operations may misinterpret the flag.Reset it after the restoration completes:
fastdom.mutate(() => { ... - this.restoreScollFlag = true + this.restoreScollFlag = true + setTimeout(() => (this.restoreScollFlag = false), 0) })Also correct the spelling for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
packages/theme-saas/src/svgs/delegated-processing.svgis excluded by!**/*.svgpackages/theme-saas/src/svgs/pushpin-solid.svgis excluded by!**/*.svgpackages/theme-saas/src/svgs/pushpin.svgis excluded by!**/*.svgpackages/theme/src/svgs/delegated-processing.svgis excluded by!**/*.svg
📒 Files selected for processing (73)
examples/sites/demos/pc/app/grid/custom/column-fixed.spec.js(1 hunks)examples/sites/demos/pc/app/grid/custom/page-size.spec.js(1 hunks)examples/sites/demos/pc/app/grid/data-source/request-service.spec.js(1 hunks)examples/sites/demos/pc/app/grid/data-source/static-data.spec.js(1 hunks)examples/sites/demos/pc/app/grid/editor/custom-edit.spec.js(1 hunks)examples/sites/demos/pc/app/grid/empty/empty-data-iscenter.spec.js(1 hunks)examples/sites/demos/pc/app/grid/event/toolbar-button-click-event.spec.js(1 hunks)examples/sites/demos/pc/app/grid/expand/set-row-expansion.spec.js(1 hunks)examples/sites/demos/pc/app/grid/filter/default-relation.spec.js(1 hunks)examples/sites/demos/pc/app/grid/filter/server-filter.spec.js(1 hunks)examples/sites/demos/pc/app/grid/footer/footer-summation-empty.spec.js(0 hunks)examples/sites/demos/pc/app/grid/large-data/full-data-loading.spec.js(2 hunks)examples/sites/demos/pc/app/grid/mouse-keyboard/keyboard-navigation.spec.js(1 hunks)examples/sites/demos/pc/app/grid/renderer/inner-renderer.spec.js(1 hunks)examples/sites/demos/pc/app/grid/size/max-min-grid-height.spec.js(1 hunks)examples/sites/demos/pc/app/grid/sort/combinations-sort.spec.js(1 hunks)examples/sites/demos/pc/app/grid/webdoc/grid-empty.js(1 hunks)examples/sites/demos/pc/app/icon/iconGroups.js(1 hunks)packages/renderless/src/grid/plugins/exportExcel.ts(1 hunks)packages/renderless/src/grid/utils/column.ts(1 hunks)packages/renderless/src/grid/utils/common.ts(4 hunks)packages/renderless/src/grid/utils/dom.ts(2 hunks)packages/theme-saas/src/grid/body.less(1 hunks)packages/theme-saas/src/grid/header.less(0 hunks)packages/theme-saas/src/grid/table.less(8 hunks)packages/theme/src/grid/body.less(1 hunks)packages/theme/src/grid/header.less(0 hunks)packages/theme/src/grid/table.less(9 hunks)packages/vue-icon-saas/index.ts(3 hunks)packages/vue-icon/index.ts(3 hunks)packages/vue-icon/src/delegated-processing/index.ts(1 hunks)packages/vue/src/grid-toolbar/src/index.ts(2 hunks)packages/vue/src/grid/index.ts(1 hunks)packages/vue/src/grid/src/adapter/src/renderer.ts(4 hunks)packages/vue/src/grid/src/body/src/body.tsx(3 hunks)packages/vue/src/grid/src/body/src/usePool.ts(1 hunks)packages/vue/src/grid/src/cell/src/cell.ts(1 hunks)packages/vue/src/grid/src/checkbox/src/methods.ts(3 hunks)packages/vue/src/grid/src/column-anchor/src/methods.ts(1 hunks)packages/vue/src/grid/src/column/src/column.ts(1 hunks)packages/vue/src/grid/src/composable/index.ts(1 hunks)packages/vue/src/grid/src/composable/useCellEvent.ts(1 hunks)packages/vue/src/grid/src/composable/useCellSpan.ts(1 hunks)packages/vue/src/grid/src/composable/useCellStatus.ts(1 hunks)packages/vue/src/grid/src/composable/useData.ts(1 hunks)packages/vue/src/grid/src/composable/useDrag/index.ts(6 hunks)packages/vue/src/grid/src/composable/useHeader.ts(1 hunks)packages/vue/src/grid/src/composable/useRowGroup.ts(1 hunks)packages/vue/src/grid/src/config.ts(3 hunks)packages/vue/src/grid/src/dragger/src/methods.ts(1 hunks)packages/vue/src/grid/src/edit/src/methods.ts(6 hunks)packages/vue/src/grid/src/fetch-data/src/methods.ts(2 hunks)packages/vue/src/grid/src/footer/index.ts(0 hunks)packages/vue/src/grid/src/footer/src/footer.ts(0 hunks)packages/vue/src/grid/src/grid/grid.ts(8 hunks)packages/vue/src/grid/src/header/index.ts(0 hunks)packages/vue/src/grid/src/header/src/header.ts(0 hunks)packages/vue/src/grid/src/keyboard/src/methods.ts(13 hunks)packages/vue/src/grid/src/menu/src/methods.ts(2 hunks)packages/vue/src/grid/src/mobile-first/index.vue(1 hunks)packages/vue/src/grid/src/pager/src/methods.ts(2 hunks)packages/vue/src/grid/src/resize/src/methods.ts(1 hunks)packages/vue/src/grid/src/table/src/methods.ts(33 hunks)packages/vue/src/grid/src/table/src/strategy.ts(4 hunks)packages/vue/src/grid/src/table/src/table.ts(18 hunks)packages/vue/src/grid/src/table/src/utils/autoCellWidth.ts(5 hunks)packages/vue/src/grid/src/table/src/utils/computeScrollLoad.ts(1 hunks)packages/vue/src/grid/src/table/src/utils/handleFixedColumn.ts(0 hunks)packages/vue/src/grid/src/table/src/utils/updateStyle.ts(1 hunks)packages/vue/src/grid/src/toolbar/src/methods.ts(1 hunks)packages/vue/src/grid/src/tools/index.ts(3 hunks)packages/vue/src/grid/src/tooltip/src/methods.ts(1 hunks)packages/vue/src/grid/src/tree/src/methods.ts(5 hunks)
💤 Files with no reviewable changes (8)
- packages/theme/src/grid/header.less
- examples/sites/demos/pc/app/grid/footer/footer-summation-empty.spec.js
- packages/vue/src/grid/src/header/index.ts
- packages/theme-saas/src/grid/header.less
- packages/vue/src/grid/src/footer/index.ts
- packages/vue/src/grid/src/header/src/header.ts
- packages/vue/src/grid/src/footer/src/footer.ts
- packages/vue/src/grid/src/table/src/utils/handleFixedColumn.ts
🧰 Additional context used
🧬 Code Graph Analysis (13)
examples/sites/demos/pc/app/grid/editor/custom-edit.spec.js (3)
examples/sites/demos/pc/app/grid/custom/column-fixed.spec.js (1)
demo(6-6)examples/sites/demos/pc/app/grid/custom/page-size.spec.js (1)
demo(6-6)examples/sites/demos/pc/app/grid/mouse-keyboard/keyboard-navigation.spec.js (1)
demo(5-5)
examples/sites/demos/pc/app/grid/custom/column-fixed.spec.js (3)
examples/sites/demos/pc/app/grid/editor/custom-edit.spec.js (1)
demo(6-6)examples/sites/demos/pc/app/grid/custom/page-size.spec.js (1)
demo(6-6)examples/sites/demos/pc/app/grid/mouse-keyboard/keyboard-navigation.spec.js (1)
demo(5-5)
packages/vue/src/grid/src/checkbox/src/methods.ts (3)
packages/renderless/src/grid/utils/common.ts (1)
getRowkey(36-36)packages/vue/src/grid-toolbar/src/index.ts (1)
vSize(313-315)packages/vue/src/grid/src/grid/grid.ts (1)
vSize(181-183)
packages/vue/src/grid-toolbar/src/index.ts (3)
packages/vue/src/grid/src/table/src/table.ts (1)
customs(526-529)packages/vue/src/grid/src/table/src/methods.ts (1)
sort(1363-1379)packages/vue/src/grid/src/grid/grid.ts (1)
columns(222-224)
packages/vue/src/grid/src/tree/src/methods.ts (2)
packages/renderless/src/grid/utils/common.ts (1)
getRowkey(36-36)packages/vue/src/grid/src/composable/useData.ts (1)
graphFullData(115-126)
packages/vue/src/grid/src/composable/useCellSpan.ts (2)
packages/vue-common/src/index.ts (1)
hooks(450-450)packages/vue/src/grid/src/table/src/strategy.ts (1)
isVirtualRow(173-173)
packages/vue/src/grid/src/composable/useDrag/index.ts (2)
packages/vue-common/src/index.ts (1)
hooks(450-450)packages/vue/src/grid/src/table/src/table.ts (2)
collectColumn(522-525)tableColumn(541-546)
packages/vue/src/grid/src/composable/useData.ts (1)
packages/renderless/src/grid/utils/common.ts (1)
getRowid(39-42)
packages/vue/src/grid/src/menu/src/methods.ts (2)
packages/vue/src/grid/src/table/src/table.ts (2)
isCtxMenu(437-439)ctxMenuOpts(425-427)packages/renderless/src/grid/utils/common.ts (1)
emitEvent(180-185)
packages/vue/src/grid/src/table/src/utils/updateStyle.ts (2)
packages/vue/src/grid/src/table/src/table.ts (3)
height(534-536)parentHeight(547-549)tableColumn(541-546)packages/renderless/src/grid/utils/dom.ts (1)
isScale(37-37)
packages/vue/src/grid/src/table/src/strategy.ts (4)
packages/vue-common/src/index.ts (1)
hooks(450-450)packages/renderless/src/grid/utils/common.ts (1)
getRowkey(36-36)packages/vue/src/grid/src/composable/useData.ts (1)
buildRenderGraph(97-100)packages/renderless/src/grid/utils/column.ts (1)
fixedDetails(135-137)
packages/renderless/src/grid/utils/dom.ts (1)
packages/renderless/src/grid/utils/common.ts (1)
getRowid(39-42)
packages/vue/src/grid/src/table/src/methods.ts (11)
packages/vue/src/grid/src/tools/index.ts (1)
error(33-33)packages/renderless/src/grid/utils/common.ts (4)
getRowkey(36-36)getRowid(39-42)getRowUniqueId(265-265)dfsCopy(233-261)packages/vue-common/src/index.ts (1)
hooks(450-450)packages/vue/src/grid/src/table/src/strategy.ts (2)
buildRowGroupFullData(173-173)sliceVisibleColumn(173-173)packages/vue/src/grid/src/composable/useData.ts (1)
buildRenderGraph(97-100)packages/vue/src/grid/src/table/src/utils/refreshColumn.ts (4)
reassignFixedRight(52-70)reassignNotFixed(46-50)showGroupFixedError(92-96)onScrollXLoad(98-118)packages/renderless/src/grid/utils/dom.ts (3)
isScale(37-37)rowToVisible(53-92)getCell(265-276)packages/vue/src/grid/src/table/src/utils/autoCellWidth.ts (2)
calcTableWidth(27-136)calcFixedDetails(227-318)packages/vue/src/grid/src/table/src/utils/updateStyle.ts (1)
handleLayout(30-73)packages/vue/src/grid/src/table/src/utils/computeScrollLoad.ts (2)
computeScrollXLoad(63-98)computeScrollYLoad(30-61)packages/vue/src/grid/src/composable/useCellStatus.ts (2)
updateRowStatus(37-39)getCellStatus(41-50)
🪛 Biome (1.9.4)
packages/vue/src/grid/src/composable/useCellSpan.ts
[error] 38-38: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/vue/src/grid/src/composable/useRowGroup.ts
[error] 77-78: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
packages/vue/src/grid/src/grid/grid.ts
[error] 331-332: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/vue/src/grid/src/keyboard/src/methods.ts
[error] 80-80: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/vue/src/grid/src/body/src/body.tsx
[error] 156-157: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 226-227: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 322-324: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 359-359: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 456-456: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 139-141: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 144-153: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 155-160: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 178-184: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 236-238: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 241-248: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 251-252: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 257-267: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 281-287: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 514-515: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 516-516: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 523-528: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 528-529: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 812-815: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 815-819: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 587-588: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 601-601: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 603-604: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 612-612: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 837-837: This aliasing of this is unnecessary.
Arrow functions inherits this from their enclosing scope.
(lint/complexity/noUselessThisAlias)
[error] 849-849: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 850-856: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 865-868: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 873-873: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 877-880: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 888-888: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 889-890: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
packages/vue/src/grid/src/table/src/methods.ts
[error] 1011-1011: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1970-1972: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (38)
examples/sites/demos/pc/app/grid/data-source/request-service.spec.js (1)
8-10: Row-count now scoped to visible rows – looks goodThe selector refinement to
.tiny-grid-body__row:visiblemakes the assertion resilient against rows that are still rendered in the DOM but hidden (e.g., viadisplay: nonefor grouping or virtual scrolling).
No concerns – change improves test stability.examples/sites/demos/pc/app/grid/filter/default-relation.spec.js (1)
9-11: Consistent visibility filter applied – OKSame visibility scoping as other updated specs; keeps expectations aligned with the user-visible state.
Change is correct.packages/renderless/src/grid/plugins/exportExcel.ts (1)
14-14: Let’s directly inspect the@opentiny/utilssource for anyextendimplementation:#!/bin/bash # Search for any export or definition of `extend` in utils rg -n "export (function|const) extend" -g "packages/utils/src/**/*.ts" # In case it's aliased internally, look for `_extend` rg -n "_extend" -g "packages/utils/src/**/*.ts"examples/sites/demos/pc/app/icon/iconGroups.js (1)
294-296: Icon added to group – duplication check passed
'IconDelegatedProcessing'is a new entry and not duplicated elsewhere in this group.
No further action needed.examples/sites/demos/pc/app/grid/custom/page-size.spec.js (1)
12-12: LGTM – selector update improves robustnessUsing
:visiblefocuses the assertion on rendered rows only and avoids false positives when rows are virtualised or hidden.examples/sites/demos/pc/app/grid/empty/empty-data-iscenter.spec.js (1)
11-11: LGTM – selector swap matches updated DOMVerifying
justify-content: centeron.tiny-grid__empty-blockreflects the new merged empty-state markup.examples/sites/demos/pc/app/grid/data-source/static-data.spec.js (1)
17-18: Selector update looks correct — keep an eye on implicit timing.Switching to the
:visiblepseudo-class prevents hidden rows from being counted. 👍
Just ensure the grid re-render has actually finished before this assertion; if you see sporadic flakiness, insert an explicit wait for the DOM mutation (e.g.await page.waitForSelector('.tiny-grid-body__row:visible')) instead of a hard timeout.packages/vue/src/grid/src/table/src/utils/computeScrollLoad.ts (1)
63-98: ```shell
#!/bin/bash
set -eSearch for occurrences of updateScrollXSpace in the repository
rg -n "updateScrollXSpace" -A 3 -B 3
</details> <details> <summary>examples/sites/demos/pc/app/grid/filter/server-filter.spec.js (1)</summary> `10-11`: **LGTM — selector narrowed to visible rows.** This mirrors updates across the suite and should eliminate false positives from hidden rows. </details> <details> <summary>packages/vue/src/grid/src/toolbar/src/methods.ts (1)</summary> `173-176`: **Parameter removed from `recalculate()` — confirm API change.** The previous call passed `true`; now it doesn’t. If `recalculate`’s signature changed, all good. Otherwise, default behaviour might differ (e.g. skipping scroll position restore). Please double-check `recalculate` implementation and update overloads/tests accordingly. </details> <details> <summary>packages/vue/src/grid/src/column/src/column.ts (1)</summary> `162-165`: ```shell #!/bin/bash grep -n "^import.*watch" -n packages/vue/src/grid/src/column/src/column.tsexamples/sites/demos/pc/app/grid/sort/combinations-sort.spec.js (1)
8-11: Expectations updated – looks goodAssertions now match the new combined-sort order and align with the earlier test-suite visibility updates.
packages/vue/src/grid/src/fetch-data/src/methods.ts (1)
39-39:resetScrollTopalways runs whenscrollis truthy/falsey mismatch.After the fix above
scrollis guaranteed boolean. Good.
However, be aware that other code paths still callresetScrollTop()un-guarded (e.g.reloadbranch). Make sure this flag is honoured consistently to avoid unexpected scroll jumps.packages/vue/src/grid/src/composable/index.ts (1)
3-7: Re-exports look good – confirm tree-shaking.Adding these composables through star-exports is fine; double-check build output to ensure they are treeshaken when unused so bundle size doesn’t regress.
packages/vue-icon/index.ts (2)
861-862: Ensure the new icon is registered in exports
TheIconDelegatedProcessingentries are added to both named and default exports. Verify downstream references (e.g.,vue-icon-saas, demo icon groups) align with these identifiers.#!/bin/bash # Check for references in saas package rg -n "DelegatedProcessing" packages/vue-icon-saas # Check examples for icon group usage rg -n "IconDelegatedProcessing" examples/sites/demosAlso applies to: 1746-1746
93-93: Validate new icon import
Confirm that thedelegated-processingmodule exists and properly exports a Vue component to avoid runtime import failures.#!/bin/bash # Verify the delegated-processing icon module exists and exports default fd -e ts delegated-processing packages/vue-icon/src || { echo "Error: delegated-processing module not found"; exit 1; } grep -R "export default" packages/vue-icon/src/delegated-processing/index.tspackages/vue/src/grid/src/cell/src/cell.ts (1)
397-407: Cleaner params – but keep an eye onseqsourceGood call dropping the unused
$seq.
seqis still relied on (startIndex + seq), so ensure every call site (e.g.genParams) still passes it; anundefinedhere would emitNaNin the index column.packages/vue/src/grid/index.ts (1)
88-90: Optional-chaining prevents hard crashSwitching to
tinyTable[name]?.apply(...)safely no-ops when the wrapped method is absent – perfect for plugin variability.
No functional issues spotted. 👍packages/vue-icon/src/delegated-processing/index.ts (1)
12-15: Consistent icon factory pattern – looks goodThe new SVG follows the established helper pattern; no issues detected.
packages/vue/src/grid/src/resize/src/methods.ts (2)
40-41: Good use of optional chaining
this.$resize?.disconnect()is concise and null-safe.
32-37: ```shell
#!/bin/bashDisplay the full content of methods.ts to understand updateParentHeight and recalculate
sed -n '1,200p' packages/vue/src/grid/src/resize/src/methods.ts
</details> <details> <summary>packages/vue/src/grid/src/pager/src/methods.ts (1)</summary> `40-60`: **Inline style removal is fine – verify CSS still centres the pager** The `style` prop was dropped; now only `ref: 'pager'` remains. Confirm that the surrounding CSS (theme packages) provides the expected `justify-content` / `display` behaviour, especially in mobile break-points where the previous inline logic handled stacking & centring. </details> <details> <summary>packages/vue/src/grid/src/tools/index.ts (1)</summary> `27-27`: **Importing `hooks` just for `toRaw` is acceptable** Good call importing `hooks` so the map keys use raw objects, avoiding identity mismatches caused by proxy wrappers. </details> <details> <summary>packages/vue/src/grid-toolbar/src/index.ts (2)</summary> `558-561`: **Optional chaining swallows update of `tableFullColumn`** If `reloadCustoms` returns a non-promise (or nothing), the `.then()` branch – which updates `tableFullColumn` – is skipped, leaving the toolbar out-of-sync. Verify that `reloadCustoms` always returns a Promise; otherwise keep the earlier synchronous assignment fallback. --- `670-674`: **`applySettings` doesn’t wait for column reload** `this.$grid.reloadCustoms` is invoked without awaiting its Promise, so subsequent operations may run before columns are ready (e.g. immediate resize/recalculate jobs elsewhere). Consider: ```diff - this.$grid.reloadCustoms(columns, sort, colWidth) + await this.$grid.reloadCustoms(columns, sort, colWidth)(or return the Promise to the caller).
packages/renderless/src/grid/utils/column.ts (1)
69-71: Setter keeps_fixedDetailsin sync – good jobThe lazily-recreated
FixedDetailsobject in thefixedsetter neatly prevents stale positioning metadata.packages/vue/src/grid/src/config.ts (1)
44-45: DefaultshowStatus: truetrades performance for UXCell-dirty tracking now runs for every edit by default.
On very large tables this can be a noticeable overhead. Consider:// keep feature opt-in for existing installations editConfig: { trigger: 'click', mode: 'cell', showStatus: false },or document the potential impact in the release notes.
packages/vue/src/grid/src/edit/src/methods.ts (1)
159-160: ```bash
#!/bin/bashecho "Searching for updateCache definition in packages/vue:"
rg -n "updateCache(" -g "*.ts" -C5 packages/vueecho
echo "Searching for updateCache definition in packages/renderless:"
rg -n "updateCache(" -g "*.ts" -C5 packages/renderless</details> <details> <summary>packages/theme/src/grid/table.less (2)</summary> `1209-1260`: **Sticky header/footer rules increase paint containment – verify on Safari** The new `.tiny-grid__body` rules rely on `contain: layout;` and multiple `position: sticky` nodes. Safari 14/15 had bugs with `contain + sticky` causing cells to disappear when scrolled. Run the visual regression tests under Safari (or WebKit GTK) before shipping to avoid a production surprise. --- `722-746`: **`.tiny-grid__empty-block` now `position: sticky` – good UX** The empty-state banner sticks nicely during vertical scroll; the flex layout keeps it centred. Nice tidy improvement. </details> <details> <summary>packages/vue/src/grid/src/composable/useDrag/index.ts (1)</summary> `192-194`: **`colidMap` may be `null` on first render** `createTableColumnWatch` is triggered before `colidMap` is populated, which can throw inside `dragStart/drop`. Add `{ immediate: true }` to the `watch` so the map is built once up-front. ```diff -hooks.watch(collectColumn, () => { +hooks.watch( + collectColumn, + () => { state.colidMap = getColidMap(state.collectColumn) -}) + }, + { immediate: true } +)packages/vue/src/grid/src/composable/useCellEvent.ts (1)
256-262: Event listeners use capture; verify necessityAll handlers are registered with
capture=true. Capture listeners run before Vue’s own bubbling handlers and can degrade performance on huge tables. If not strictly required, prefer bubbling (defaultfalse).packages/vue/src/grid/src/table/src/utils/autoCellWidth.ts (1)
28-44: Width calculation uses initialclientWidthbefore deductions
meanWidthfor percentage/min columns is derived fromremainWidthwhich is still the full
clientWidthat this point, so laterpxMin/pxdeductions make those columns wider than intended
when horizontal scrollbar is present. Consider postponingmeanWidthcalculation until after the
fixed-width loops or recompute onceremainWidthis updated.packages/renderless/src/grid/utils/dom.ts (1)
68-77:map.get(row)can be undefinedWhen
rowis no longer present in_tileInfo.map,indexOfreturns-1, makingposition
negative and scrolling the wrong way.
Add a guard:const node = map.get(row) if (!node) returnpackages/vue/src/grid/src/body/src/body.tsx (2)
156-157: Use optional chaining for cleaner code.- hasDefaultTip && validError ? ( + hasDefaultTip && validError && (Though actually, looking at the code more carefully, the current ternary with null fallback is appropriate here for JSX rendering.
737-749: Store ResizeObserver instance for proper cleanup.The ResizeObserver is created but not stored as an instance variable, which could lead to memory leaks if the component is destroyed while observations are active.
+ const resizeObserverRef = hooks.ref<ResizeObserver>() + const resizeObserver = new ResizeObserver((entries) => { for (let entry of entries) { const target = entry.target as HTMLElement switch (target) { case body.value: bodyClientWidth.value = target.clientWidth break case customFooter.value: $table.footerHeight = target.offsetHeight } } }) + resizeObserverRef.value = resizeObserverThen update the cleanup in onBeforeUnmount:
- resizeObserver.disconnect() + resizeObserverRef.value?.disconnect()Likely an incorrect or invalid review comment.
packages/vue/src/grid/src/table/src/table.ts (2)
741-745: Document the purpose of appending table wrapper to body.Appending the table wrapper directly to document.body is an unusual pattern that could cause issues with component encapsulation and styling.
Please clarify why this DOM manipulation is necessary. If it's for positioning (like modals/tooltips), consider using a portal/teleport pattern instead:
// Vue 3 Teleport example <Teleport to="body" :disabled="!shouldTeleport"> <div ref="tableWrapper">...</div> </Teleport>
551-827: Well-executed migration to Vue 3 Composition API.The refactoring from Vue 2 Options API to Vue 3 Composition API is comprehensive and well-implemented. Good use of:
- Reactive refs and computed properties
- Proper lifecycle hooks
- Composables for modular functionality
- TypeScript for better type safety
| export function computeScrollYLoad({ _vm, scrollLoad, scrollY, scrollYLoad, scrollYStore, tableBodyElem }) { | ||
| if (scrollYLoad || scrollLoad) { | ||
| // 获取表格体默认第一行的高度 | ||
| scrollYStore.rowHeight = _vm.getRowHeight() | ||
| scrollYStore.rowHeight = _vm.rowHeight | ||
| } | ||
|
|
There was a problem hiding this comment.
Potential divide-by-zero if rowHeight not initialised.
scrollYStore.rowHeight = _vm.rowHeight assumes the reactive rowHeight is already populated.
If it’s still 0 / undefined, later Math.ceil(bodyHeight / scrollYStore.rowHeight) will blow up.
- scrollYStore.rowHeight = _vm.rowHeight
+ scrollYStore.rowHeight = _vm.rowHeight || _vm.getRowHeight?.() || 1 // fallbackAt minimum, guard against falsy values or fall back to the old DOM query.
📝 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.
| export function computeScrollYLoad({ _vm, scrollLoad, scrollY, scrollYLoad, scrollYStore, tableBodyElem }) { | |
| if (scrollYLoad || scrollLoad) { | |
| // 获取表格体默认第一行的高度 | |
| scrollYStore.rowHeight = _vm.getRowHeight() | |
| scrollYStore.rowHeight = _vm.rowHeight | |
| } | |
| export function computeScrollYLoad({ _vm, scrollLoad, scrollY, scrollYLoad, scrollYStore, tableBodyElem }) { | |
| if (scrollYLoad || scrollLoad) { | |
| // 获取表格体默认第一行的高度 | |
| - scrollYStore.rowHeight = _vm.rowHeight | |
| + scrollYStore.rowHeight = _vm.rowHeight || _vm.getRowHeight?.() || 1 // fallback | |
| } | |
| // …rest of implementation… | |
| } |
🤖 Prompt for AI Agents
In packages/vue/src/grid/src/table/src/utils/computeScrollLoad.ts around lines
30 to 35, the assignment of scrollYStore.rowHeight from _vm.rowHeight can lead
to a divide-by-zero error if rowHeight is zero or undefined. To fix this, add a
check to ensure _vm.rowHeight is a valid positive number before assigning it; if
not, fall back to querying the DOM for the row height as a default value. This
prevents errors in subsequent calculations using scrollYStore.rowHeight.
| await page.goto('grid-custom#custom-column-fixed') | ||
| await page.locator('.tiny-grid-custom__setting-btn').click() | ||
| await custom.getByRole('row', { name: '员工数 ' }).getByTitle('未冻结').getByRole('img').click() | ||
| await custom.getByRole('row', { name: '员工数' }).getByTitle('左冻结').getByRole('img').click() | ||
| await page.getByRole('button', { name: '确定' }).click() | ||
| await expect(page.getByRole('cell', { name: '员工数' })).toHaveCSS('right', '0px') | ||
| await expect(demo.locator('.tiny-grid-header__row th').nth(3)).toHaveText(/员工数/) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Assertion no longer validates “fixed” behaviour – only text.
The previous test verified the header cell’s right style to ensure the column is actually fixed.
By switching to a pure text check we lose coverage of the sticky-column behaviour that this demo is supposed to showcase. At minimum, assert that the cell has the expected CSS class (col__fixed) or computed style (position: sticky, left/right offset), e.g.:
-await expect(demo.locator('.tiny-grid-header__row th').nth(3)).toHaveText(/员工数/)
+const target = demo.locator('.tiny-grid-header__row th').nth(3)
+await expect(target).toHaveText(/员工数/)
+await expect(target).toHaveClass(/col__fixed/) // or style check📝 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.
| await expect(demo.locator('.tiny-grid-header__row th').nth(3)).toHaveText(/员工数/) | |
| // replace the lone text assertion with these lines to also verify the fixed column | |
| const target = demo.locator('.tiny-grid-header__row th').nth(3) | |
| await expect(target).toHaveText(/员工数/) | |
| await expect(target).toHaveClass(/col__fixed/) |
🤖 Prompt for AI Agents
In examples/sites/demos/pc/app/grid/custom/column-fixed.spec.js at line 12, the
test only checks the header cell text and does not verify the fixed column
behavior. Update the assertion to also check that the header cell has the CSS
class 'col__fixed' or verify its computed style includes 'position: sticky' with
the appropriate left or right offset to confirm the column is fixed as intended.
| let { reloadConfig = {} } = fetchData | ||
| let { scroll = false } = reloadConfig | ||
|
|
There was a problem hiding this comment.
Destructuring fetchData when it may be undefined causes a runtime crash.
If consumers only provide a dataset, fetchData is undefined, making
let { reloadConfig = {} } = fetchData throw TypeError: Cannot destructure property 'reloadConfig' of 'fetchData' as it is undefined.
Proposed fix:
-let { reloadConfig = {} } = fetchData
-let { scroll = false } = reloadConfig
+const reloadConfig = fetchData?.reloadConfig ?? {}
+const { scroll = false } = reloadConfig📝 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.
| let { reloadConfig = {} } = fetchData | |
| let { scroll = false } = reloadConfig | |
| const reloadConfig = fetchData?.reloadConfig ?? {} | |
| const { scroll = false } = reloadConfig |
🤖 Prompt for AI Agents
In packages/vue/src/grid/src/fetch-data/src/methods.ts around lines 28 to 30,
the code destructures reloadConfig from fetchData without checking if fetchData
is undefined, causing a runtime crash. To fix this, add a default empty object
when destructuring fetchData, like using let { reloadConfig = {} } = fetchData
|| {}, so that if fetchData is undefined, it falls back to an empty object and
prevents the error.
| @@ -5,7 +5,7 @@ export default { | |||
| // 处理列拖拽 | |||
| columnDrop(headerEl) { | |||
| const { plugin, onBeforeMove, filter } = this.dropConfig || {} | |||
| const columnDropContainer = headerEl.querySelector('.tiny-grid__header .tiny-grid-header__row') | |||
| const columnDropContainer = headerEl.querySelector('.tiny-grid-header__row') | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Selector assumes a single .tiny-grid-header__row; adds null-risk.
headerEl.querySelector('.tiny-grid-header__row') will return null if the header uses multiple rows or a different template, leading to plugin.create throwing.
Defensive guard recommended:
-const columnDropContainer = headerEl.querySelector('.tiny-grid-header__row')
+const columnDropContainer = headerEl.querySelector('.tiny-grid-header__row')
+if (!columnDropContainer) return // or throw descriptive error📝 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.
| const columnDropContainer = headerEl.querySelector('.tiny-grid-header__row') | |
| const columnDropContainer = headerEl.querySelector('.tiny-grid-header__row') | |
| if (!columnDropContainer) return // or throw a descriptive error |
🤖 Prompt for AI Agents
In packages/vue/src/grid/src/dragger/src/methods.ts at line 8, the code assumes
querySelector('.tiny-grid-header__row') always returns an element, but it can
return null if multiple rows or different templates are used. Add a defensive
check to verify columnDropContainer is not null before using it, and handle the
null case appropriately to prevent plugin.create from throwing errors.
| this._delayActivateAnchor = () => { | ||
| this._delayActivateAnchor = undefined | ||
| setTimeout(() => this.anchorAction({ field: activeAnchor.field, anchors, _vm: this }), activeAnchor.delay) | ||
| } |
There was a problem hiding this comment.
_delayActivateAnchor is created but never invoked.
buildColumnAnchorParams now defines _delayActivateAnchor, yet no call site exists in this file.
If external code is expected to call it, expose it clearly; otherwise schedule it here:
this._delayActivateAnchor = () => {
this._delayActivateAnchor = undefined
setTimeout(() => this.anchorAction({ field: activeAnchor.field, anchors, _vm: this }), activeAnchor.delay)
}
+// Trigger once after build
+this._delayActivateAnchor()Also consider clearing any previous timer to avoid leaks when the grid is rebuilt.
📝 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.
| this._delayActivateAnchor = () => { | |
| this._delayActivateAnchor = undefined | |
| setTimeout(() => this.anchorAction({ field: activeAnchor.field, anchors, _vm: this }), activeAnchor.delay) | |
| } | |
| this._delayActivateAnchor = () => { | |
| this._delayActivateAnchor = undefined | |
| setTimeout(() => this.anchorAction({ field: activeAnchor.field, anchors, _vm: this }), activeAnchor.delay) | |
| } | |
| // Trigger once after build | |
| this._delayActivateAnchor() |
🤖 Prompt for AI Agents
In packages/vue/src/grid/src/column-anchor/src/methods.ts around lines 95 to 98,
the function _delayActivateAnchor is defined but never called, which means the
intended delayed activation does not occur. To fix this, either invoke
_delayActivateAnchor immediately after defining it if it should run internally,
or if it is meant to be called externally, expose it clearly as part of the
component's public API. Additionally, implement logic to clear any existing
timeout before setting a new one to prevent timer leaks when the grid is
rebuilt.
| const { $grid, elemStore, scrollLoad, scrollLoadStore, headerHeight, footerHeight, rowHeight } = this | ||
| const totalRows = this.getRowCount() | ||
| const scrollHeight = $grid.pagerConfig ? $grid.pagerConfig.total * rowHeight : 0 | ||
| const ySpaceElem = elemStore[`main-body-ySpace`] | ||
|
|
||
| // 滚动分页场景的视口高度和滚动高度缓存 | ||
| scrollLoadStore.bodyHeight = bodyHeight | ||
| scrollLoadStore.bodyHeight = totalRows * rowHeight + headerHeight + footerHeight | ||
| scrollLoadStore.scrollHeight = scrollHeight | ||
|
|
||
| const tableElem = elemStore['main-body-table'] | ||
|
|
||
| // 这里最好使用transform3D,使用gpu加速，防止页面重绘 | ||
| if (tableElem) { | ||
| tableElem.style.transform = marginTop ? `translateY(${marginTop})` : '' | ||
| } | ||
|
|
||
| const ySpaceElem = elemStore['main-body-ySpace'] | ||
| ySpaceElem && (ySpaceElem.style.height = ySpaceHeight) | ||
|
|
||
| // 滚动分页加载逻辑 | ||
| if (ySpaceElem && scrollLoad && $grid) { | ||
| Object.assign(scrollLoadStore, { bodyHeight, scrollHeight }) | ||
| ySpaceElem.firstChild.style.height = `${scrollHeight}px` | ||
| ySpaceElem.onscroll = this.debounceScrollLoad | ||
| if (ySpaceElem && scrollLoad) { | ||
| // 更新滚动分页场景的滚动高度 | ||
| this.scrollLoadScrollHeight = scrollHeight | ||
| } |
There was a problem hiding this comment.
updateScrollYSpace – incorrect total height without pager
scrollHeight is set to 0 when pagerConfig is absent, which collapses the scroll-load bar. It should fall back to totalRows * rowHeight just like bodyHeight.
const scrollHeight =
$grid.pagerConfig ? $grid.pagerConfig.total * rowHeight : totalRows * rowHeight📝 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.
| const { $grid, elemStore, scrollLoad, scrollLoadStore, headerHeight, footerHeight, rowHeight } = this | |
| const totalRows = this.getRowCount() | |
| const scrollHeight = $grid.pagerConfig ? $grid.pagerConfig.total * rowHeight : 0 | |
| const ySpaceElem = elemStore[`main-body-ySpace`] | |
| // 滚动分页场景的视口高度和滚动高度缓存 | |
| scrollLoadStore.bodyHeight = bodyHeight | |
| scrollLoadStore.bodyHeight = totalRows * rowHeight + headerHeight + footerHeight | |
| scrollLoadStore.scrollHeight = scrollHeight | |
| const tableElem = elemStore['main-body-table'] | |
| // 这里最好使用transform3D,使用gpu加速，防止页面重绘 | |
| if (tableElem) { | |
| tableElem.style.transform = marginTop ? `translateY(${marginTop})` : '' | |
| } | |
| const ySpaceElem = elemStore['main-body-ySpace'] | |
| ySpaceElem && (ySpaceElem.style.height = ySpaceHeight) | |
| // 滚动分页加载逻辑 | |
| if (ySpaceElem && scrollLoad && $grid) { | |
| Object.assign(scrollLoadStore, { bodyHeight, scrollHeight }) | |
| ySpaceElem.firstChild.style.height = `${scrollHeight}px` | |
| ySpaceElem.onscroll = this.debounceScrollLoad | |
| if (ySpaceElem && scrollLoad) { | |
| // 更新滚动分页场景的滚动高度 | |
| this.scrollLoadScrollHeight = scrollHeight | |
| } | |
| const { $grid, elemStore, scrollLoad, scrollLoadStore, headerHeight, footerHeight, rowHeight } = this | |
| const totalRows = this.getRowCount() | |
| - const scrollHeight = $grid.pagerConfig ? $grid.pagerConfig.total * rowHeight : 0 | |
| + const scrollHeight = $grid.pagerConfig | |
| + ? $grid.pagerConfig.total * rowHeight | |
| + : totalRows * rowHeight | |
| const ySpaceElem = elemStore[`main-body-ySpace`] | |
| // 滚动分页场景的视口高度和滚动高度缓存 | |
| scrollLoadStore.bodyHeight = totalRows * rowHeight + headerHeight + footerHeight | |
| scrollLoadStore.scrollHeight = scrollHeight | |
| if (ySpaceElem && scrollLoad) { | |
| // 更新滚动分页场景的滚动高度 | |
| this.scrollLoadScrollHeight = scrollHeight | |
| } |
🤖 Prompt for AI Agents
In packages/vue/src/grid/src/table/src/methods.ts around lines 1661 to 1673, the
scrollHeight is incorrectly set to 0 when pagerConfig is absent, causing the
scroll-load bar to collapse. Modify the assignment of scrollHeight to default to
totalRows multiplied by rowHeight if pagerConfig is not present, ensuring it
matches the calculation used for bodyHeight and maintains correct scroll height.
| // 在表格列就绪后，才处理数据过滤排序、保留选中、表头选中、多选禁用、默认状态 | ||
| if (force && this.tableFullColumn?.length > 0) { | ||
| // 对表格全量数据进行过滤排序得到后全量数据，计算分组表数据，生成图形数据 | ||
| this.updateAfterFullData() | ||
| // selectConfig.reserve保留多选状态，使用后全量数据计算表头多选状态 | ||
| this.updateSelectionStatus() | ||
| // 处理表头多选禁用 | ||
| this.handleSelectionHeader() | ||
| } | ||
|
|
||
| // 判断是否有虚拟滚动，有即剪切数据 | ||
| this.tableData = sliceFullData(this) | ||
| const { scrollYStore, _graphInfo, scrollYLoad } = this | ||
| const { renderSize, startIndex } = scrollYStore | ||
| const graphed = _graphInfo?.graphed | ||
| const tableNode = scrollYLoad ? graphed?.slice(startIndex, startIndex + renderSize) : graphed | ||
| const tableData = tableNode?.map((node) => node.payload) | ||
|
|
||
| this.tableNode = tableNode | ||
| this.tableData = tableData | ||
|
|
There was a problem hiding this comment.
Missing null-safety around _graphInfo
tableNode is derived from _graphInfo?.graphed, but if updateAfterFullData() hasn’t run (e.g. component mounted with force === false) graphed is undefined, causing tableData to be undefined and later loops (updateScrollStatus, view templates) to crash.
Guard against it:
-const tableData = tableNode?.map((node) => node.payload)
+const tableData = (tableNode || []).map((node) => node.payload)or bail out early when _graphInfo is absent.
📝 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.
| // 在表格列就绪后，才处理数据过滤排序、保留选中、表头选中、多选禁用、默认状态 | |
| if (force && this.tableFullColumn?.length > 0) { | |
| // 对表格全量数据进行过滤排序得到后全量数据，计算分组表数据，生成图形数据 | |
| this.updateAfterFullData() | |
| // selectConfig.reserve保留多选状态，使用后全量数据计算表头多选状态 | |
| this.updateSelectionStatus() | |
| // 处理表头多选禁用 | |
| this.handleSelectionHeader() | |
| } | |
| // 判断是否有虚拟滚动，有即剪切数据 | |
| this.tableData = sliceFullData(this) | |
| const { scrollYStore, _graphInfo, scrollYLoad } = this | |
| const { renderSize, startIndex } = scrollYStore | |
| const graphed = _graphInfo?.graphed | |
| const tableNode = scrollYLoad ? graphed?.slice(startIndex, startIndex + renderSize) : graphed | |
| const tableData = tableNode?.map((node) => node.payload) | |
| this.tableNode = tableNode | |
| this.tableData = tableData | |
| // 在表格列就绪后，才处理数据过滤排序、保留选中、表头选中、多选禁用、默认状态 | |
| if (force && this.tableFullColumn?.length > 0) { | |
| // 对表格全量数据进行过滤排序得到后全量数据，计算分组表数据，生成图形数据 | |
| this.updateAfterFullData() | |
| // selectConfig.reserve保留多选状态，使用后全量数据计算表头多选状态 | |
| this.updateSelectionStatus() | |
| // 处理表头多选禁用 | |
| this.handleSelectionHeader() | |
| } | |
| const { scrollYStore, _graphInfo, scrollYLoad } = this | |
| const { renderSize, startIndex } = scrollYStore | |
| const graphed = _graphInfo?.graphed | |
| const tableNode = scrollYLoad | |
| ? graphed?.slice(startIndex, startIndex + renderSize) | |
| : graphed | |
| - const tableData = tableNode?.map((node) => node.payload) | |
| + const tableData = (tableNode || []).map((node) => node.payload) | |
| this.tableNode = tableNode | |
| this.tableData = tableData |
🤖 Prompt for AI Agents
In packages/vue/src/grid/src/table/src/methods.ts around lines 258 to 276, the
code uses _graphInfo?.graphed without ensuring _graphInfo is defined, which can
cause tableData to be undefined and lead to crashes later. To fix this, add a
null check to ensure _graphInfo and _graphInfo.graphed are defined before using
them, or return early from the method if _graphInfo is absent to prevent further
processing with undefined data.
| let { $refs, editRules, tableData, validStore } = this | ||
| let { tableBody } = $refs | ||
| if (!scope || !tableBody) { | ||
| return this.$nextTick() | ||
| } | ||
|
|
||
| if (renderOpts && renderOpts.isValidAlways) { | ||
| validStore.visible = true | ||
| let { column, row } = scope | ||
| let type = 'change' | ||
|
|
||
| let refreshStatus = () => { | ||
| if (!isUndefined(cellValue)) { | ||
| this.updateRowStatus(row) | ||
| this.$refs.tableBody?.$forceUpdate() | ||
| } | ||
| } | ||
| // 如果没有相关校验规则，直接返回 | ||
| if (!editRules || !this.hasCellRules(type, row, column)) { | ||
| refreshStatus() | ||
| return this.$nextTick() | ||
| } | ||
|
|
||
| let { column, row } = scope | ||
| let type = 'change' | ||
| if (!this.hasCellRules(type, row, column)) { | ||
| // 如果设置了始终验证 | ||
| if (renderOpts && renderOpts.isValidAlways) { | ||
| validStore.visible = true | ||
| } | ||
|
|
||
| // 获取单元格元素并执行校验 | ||
| let rowIndex = tableData.indexOf(row) | ||
| getCell(this, { row, rowIndex, column }).then((cell) => { | ||
| if (!cell) { | ||
| return | ||
| } | ||
| let rowIndex = tableData.indexOf(row) | ||
| getCell(this, { row, rowIndex, column }).then((cell) => { | ||
| if (!cell) { | ||
| return | ||
| } | ||
| return this.validCellRules(type, row, column, cellValue) | ||
| .then(() => { | ||
| customValue && validStore.visible && setCellValue(row, column, cellValue) | ||
| this.clearValidate() | ||
| }) | ||
| .catch(({ rule }) => { | ||
| customValue && setCellValue(row, column, cellValue) | ||
| this.showValidTooltip({ rule, row, column, cell }) | ||
| }) | ||
| }) | ||
| return this.validCellRules(type, row, column, cellValue) | ||
| .then(() => { | ||
| // 校验通过，设置新值并清除验证提示 | ||
| refreshStatus() | ||
| this.clearValidate() | ||
| }) | ||
| .catch(({ rule }) => { | ||
| refreshStatus() | ||
| this.showValidTooltip({ rule, row, column, cell }) | ||
| }) | ||
| }) | ||
|
|
||
| return this.$nextTick() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
updateStatus – potential race & missing await
getCell(...).then(...) is not awaited, the outer function resolves before validation finishes, so callers cannot determine when the cell is actually updated/validated.
Return the promise:
-return this.$nextTick()
+return getCell(...).then(/* existing chain */)Also, rowIndex is computed from tableData that may mutate between async ticks, leading to wrong DOM lookup. Capture row’s current index once or use querySelector with data-rowid.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/vue/src/grid/src/table/src/methods.ts around lines 1817 to 1861, the
asynchronous call to getCell is not awaited, causing the outer function to
resolve before validation completes, which prevents callers from knowing when
the update finishes. Modify the function to return the promise chain from
getCell and await it properly. Additionally, capture the row index once before
the async call to avoid inconsistencies if tableData mutates during the async
operation, or alternatively use a stable selector like data-rowid for DOM
lookup.
| /** 设置数据查找缓存，对数据进行备份，深度克隆 */ | ||
| updateCache(backup = false, deepCopy = false) { | ||
| const { tableFullData, treeConfig, treeOrdered } = this | ||
| const rowKey = getRowkey(this) | ||
| const { children: childrenKey, temporaryIndex = '_$index_' } = treeConfig || {} | ||
| const isTreeOrderedFalse = treeConfig && !treeOrdered | ||
| const backupMap = new WeakMap() | ||
|
|
||
| this.fullDataRowIdData = {} | ||
| this.fullDataRowMap.clear() | ||
|
|
||
| const copyRow = (row, index, parent) => { | ||
| let rowId = getRowid(this, row) | ||
| if (isNull(rowId) || rowId === '') { | ||
|
|
||
| if (!rowId) { | ||
| rowId = getRowUniqueId() | ||
| set(row, rowKey, rowId) | ||
| } | ||
| let rowCache = { row, rowid: rowId, index } | ||
| if (source) { | ||
| fullDataRowIdData[rowId] = rowCache | ||
| fullDataRowMap.set(row, rowCache) | ||
| } | ||
| fullAllDataRowIdData[rowId] = rowCache | ||
| fullAllDataRowMap.set(row, rowCache) | ||
| } | ||
|
|
||
| let clearCache = () => { | ||
| fullAllDataRowIdData = {} | ||
| this.fullAllDataRowIdData = fullAllDataRowIdData | ||
| fullAllDataRowMap.clear() | ||
| if (source) { | ||
| fullDataRowIdData = {} | ||
| this.fullDataRowIdData = fullDataRowIdData | ||
| fullDataRowMap.clear() | ||
|
|
||
| const rowCache = { row, rowid: rowId, index } | ||
|
|
||
| this.fullDataRowIdData[rowId] = rowCache | ||
| this.fullDataRowMap.set(hooks.toRaw(row), rowCache) | ||
|
|
||
| if (backup) { | ||
| if (isTreeOrderedFalse) { | ||
| let parentIndex | ||
|
|
||
| if (parent) { | ||
| parentIndex = get(parent, temporaryIndex) | ||
| } | ||
|
|
||
| set(row, temporaryIndex, (parentIndex ? `${parentIndex}.` : '') + (index + 1)) | ||
| } | ||
|
|
||
| const childrenField = treeConfig ? { [childrenKey]: undefined } : {} | ||
| const backupRow = deepCopy ? clone({ ...row, ...childrenField }, true) : { ...row, ...childrenField } | ||
|
|
||
| backupMap.set(row, backupRow) | ||
|
|
||
| return backupRow | ||
| } | ||
| } | ||
| clearCache() | ||
| if (treeConfig) { | ||
| eachTree(tableFullData, buildRowCache, treeConfig) | ||
| } else { | ||
| tableFullData.forEach(buildRowCache) | ||
|
|
||
| const backupData = dfsCopy(tableFullData, copyRow, undefined, treeConfig, childrenKey) | ||
|
|
||
| if (backup) { | ||
| Object.assign(this, { tableSourceData: backupData, backupMap }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
updateCache – parameter misuse & row mutation
dfsCopy(tableFullData, copyRow, undefined, treeConfig, childrenKey)passes the wholetreeConfigobject to the booleanisTreeparameter. Use!!treeConfigto avoid relying on truthy coercion.copyRowmutates the originalrowby injecting a generated row key even when only a readonly backup is expected. Down-stream consumers that rely on pristine data (e.g. diffing) will be surprised.
-const backupData = dfsCopy(tableFullData, copyRow, undefined, treeConfig, childrenKey)
+const backupData = dfsCopy(tableFullData, copyRow, undefined, !!treeConfig, childrenKey)Consider cloning before mutating or generating keys lazily.
📝 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.
| /** 设置数据查找缓存，对数据进行备份，深度克隆 */ | |
| updateCache(backup = false, deepCopy = false) { | |
| const { tableFullData, treeConfig, treeOrdered } = this | |
| const rowKey = getRowkey(this) | |
| const { children: childrenKey, temporaryIndex = '_$index_' } = treeConfig || {} | |
| const isTreeOrderedFalse = treeConfig && !treeOrdered | |
| const backupMap = new WeakMap() | |
| this.fullDataRowIdData = {} | |
| this.fullDataRowMap.clear() | |
| const copyRow = (row, index, parent) => { | |
| let rowId = getRowid(this, row) | |
| if (isNull(rowId) || rowId === '') { | |
| if (!rowId) { | |
| rowId = getRowUniqueId() | |
| set(row, rowKey, rowId) | |
| } | |
| let rowCache = { row, rowid: rowId, index } | |
| if (source) { | |
| fullDataRowIdData[rowId] = rowCache | |
| fullDataRowMap.set(row, rowCache) | |
| } | |
| fullAllDataRowIdData[rowId] = rowCache | |
| fullAllDataRowMap.set(row, rowCache) | |
| } | |
| let clearCache = () => { | |
| fullAllDataRowIdData = {} | |
| this.fullAllDataRowIdData = fullAllDataRowIdData | |
| fullAllDataRowMap.clear() | |
| if (source) { | |
| fullDataRowIdData = {} | |
| this.fullDataRowIdData = fullDataRowIdData | |
| fullDataRowMap.clear() | |
| const rowCache = { row, rowid: rowId, index } | |
| this.fullDataRowIdData[rowId] = rowCache | |
| this.fullDataRowMap.set(hooks.toRaw(row), rowCache) | |
| if (backup) { | |
| if (isTreeOrderedFalse) { | |
| let parentIndex | |
| if (parent) { | |
| parentIndex = get(parent, temporaryIndex) | |
| } | |
| set(row, temporaryIndex, (parentIndex ? `${parentIndex}.` : '') + (index + 1)) | |
| } | |
| const childrenField = treeConfig ? { [childrenKey]: undefined } : {} | |
| const backupRow = deepCopy ? clone({ ...row, ...childrenField }, true) : { ...row, ...childrenField } | |
| backupMap.set(row, backupRow) | |
| return backupRow | |
| } | |
| } | |
| clearCache() | |
| if (treeConfig) { | |
| eachTree(tableFullData, buildRowCache, treeConfig) | |
| } else { | |
| tableFullData.forEach(buildRowCache) | |
| const backupData = dfsCopy(tableFullData, copyRow, undefined, treeConfig, childrenKey) | |
| if (backup) { | |
| Object.assign(this, { tableSourceData: backupData, backupMap }) | |
| } | |
| const backupData = dfsCopy( | |
| tableFullData, | |
| copyRow, | |
| undefined, | |
| - treeConfig, | |
| + !!treeConfig, | |
| childrenKey | |
| ) | |
| if (backup) { | |
| Object.assign(this, { tableSourceData: backupData, backupMap }) | |
| } |
🤖 Prompt for AI Agents
In packages/vue/src/grid/src/table/src/methods.ts around lines 359 to 407, the
call to dfsCopy incorrectly passes the entire treeConfig object to a parameter
expecting a boolean isTree; fix this by passing !!treeConfig instead. Also, the
copyRow function mutates the original row by setting a generated row key even
when only a readonly backup is needed; to fix this, avoid mutating the original
row directly by cloning it first before adding keys or generate keys lazily
without modifying the original data.
| const Methods = { | ||
| // 监听某个元素是否出现在视口中 | ||
| addIntersectionObserver() { | ||
| if (typeof IntersectionObserver === 'undefined') { | ||
| return | ||
| } | ||
|
|
||
| // 深拷贝 | ||
| const backupRow = clone({ ...row, [children]: null }, true) | ||
| this.intersectionObserver = new IntersectionObserver((entries) => { | ||
| let entry = entries[0] | ||
|
|
||
| backup.push(backupRow) | ||
| backupMap.set(row, backupRow) | ||
| if (entries.length > 1) { | ||
| const intersectingEntry = entries.find((entry) => entry.isIntersecting) | ||
|
|
||
| if (row[children]) { | ||
| backupRow[children] = traverse(row[children], rowLevel + 1, isTreeOrderedFalse ? row[temporaryIndex] : '') | ||
| if (intersectingEntry) { | ||
| entry = intersectingEntry | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| return backup | ||
| } | ||
|
|
||
| const backupData = traverse(tableData, 0, '') | ||
| } | ||
|
|
||
| return { backupData, backupMap } | ||
| } | ||
| this.handleVisibilityChange(entry.isIntersecting, entry) | ||
| }, this.intersectionOption) | ||
|
|
||
| const Methods = { | ||
| this.intersectionObserver.observe(this.$refs.tableBody.$el) | ||
| }, | ||
| removeIntersectionObserver() { | ||
| if (this.intersectionObserver) { | ||
| this.intersectionObserver.unobserve(this.$refs.tableBody.$el) | ||
| this.intersectionObserver.disconnect() | ||
| this.intersectionObserver = null | ||
| } | ||
| }, |
There was a problem hiding this comment.
intersectionObserver lifecycle & option name
- The property referenced is
this.intersectionOption, but every other place in the codebase seems to use the pluralintersectionOptions. A typo here will silently disable the observer or throw at runtime. removeIntersectionObserver()is defined but never wired tobeforeUnmount; if the parent grid component is repeatedly mounted/un-mounted the observer keeps the body element alive -> memory leak.
+onBeforeUnmount() {
+ this.removeIntersectionObserver()
+},Recommend renaming to the correct option key and ensuring removeIntersectionObserver is called in beforeUnmount.
📝 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.
| const Methods = { | |
| // 监听某个元素是否出现在视口中 | |
| addIntersectionObserver() { | |
| if (typeof IntersectionObserver === 'undefined') { | |
| return | |
| } | |
| // 深拷贝 | |
| const backupRow = clone({ ...row, [children]: null }, true) | |
| this.intersectionObserver = new IntersectionObserver((entries) => { | |
| let entry = entries[0] | |
| backup.push(backupRow) | |
| backupMap.set(row, backupRow) | |
| if (entries.length > 1) { | |
| const intersectingEntry = entries.find((entry) => entry.isIntersecting) | |
| if (row[children]) { | |
| backupRow[children] = traverse(row[children], rowLevel + 1, isTreeOrderedFalse ? row[temporaryIndex] : '') | |
| if (intersectingEntry) { | |
| entry = intersectingEntry | |
| } | |
| }) | |
| } | |
| return backup | |
| } | |
| const backupData = traverse(tableData, 0, '') | |
| } | |
| return { backupData, backupMap } | |
| } | |
| this.handleVisibilityChange(entry.isIntersecting, entry) | |
| }, this.intersectionOption) | |
| const Methods = { | |
| this.intersectionObserver.observe(this.$refs.tableBody.$el) | |
| }, | |
| removeIntersectionObserver() { | |
| if (this.intersectionObserver) { | |
| this.intersectionObserver.unobserve(this.$refs.tableBody.$el) | |
| this.intersectionObserver.disconnect() | |
| this.intersectionObserver = null | |
| } | |
| }, | |
| const Methods = { | |
| // 监听某个元素是否出现在视口中 | |
| addIntersectionObserver() { | |
| if (typeof IntersectionObserver === 'undefined') { | |
| return | |
| } | |
| this.intersectionObserver = new IntersectionObserver((entries) => { | |
| let entry = entries[0] | |
| if (entries.length > 1) { | |
| const intersectingEntry = entries.find((entry) => entry.isIntersecting) | |
| if (intersectingEntry) { | |
| entry = intersectingEntry | |
| } | |
| } | |
| this.handleVisibilityChange(entry.isIntersecting, entry) | |
| }, this.intersectionOption) | |
| this.intersectionObserver.observe(this.$refs.tableBody.$el) | |
| }, | |
| removeIntersectionObserver() { | |
| if (this.intersectionObserver) { | |
| this.intersectionObserver.unobserve(this.$refs.tableBody.$el) | |
| this.intersectionObserver.disconnect() | |
| this.intersectionObserver = null | |
| } | |
| }, | |
| + onBeforeUnmount() { | |
| + this.removeIntersectionObserver() | |
| + }, | |
| } |
🤖 Prompt for AI Agents
In packages/vue/src/grid/src/table/src/methods.ts between lines 154 and 183,
rename the property this.intersectionOption to this.intersectionOptions to match
the rest of the codebase and avoid runtime errors. Additionally, ensure that
removeIntersectionObserver() is called in the component's beforeUnmount
lifecycle hook to properly clean up the observer and prevent memory leaks when
the component is unmounted.
refactor: 优化表格性能，对表格进行重构
PR
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
Enhancements
Bug Fixes
Style
Chores
Tests
Documentation