Skip to content

fix(grid): fix user passed scrollY value is null #3658

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

fix(grid): fix user passed scrollY value is null #3658

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/vue/src/grid/src/table/src/table.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,9 @@ export default defineComponent({
const optimizeOpts = hooks.computed(() => extend(true, {}, GlobalConfig.optimization, props.optimization))
const rowHeight = hooks.computed(
() =>
optimizeOpts.value?.scrollY.rHeight || GlobalConfig.rowHeight[tinyTheme.value]?.[vSize.value || 'default'] || 40
optimizeOpts.value?.scrollY?.rHeight ||
GlobalConfig.rowHeight[tinyTheme.value]?.[vSize.value || 'default'] ||
40
)
Comment on lines 711 to 716
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Add a regression test and scan for other unguarded scrollY dereferences

Given this bug class, add a test that mounts the grid with optimization: { scrollY: null } and asserts no error and that rowHeight resolves to theme/default values. Also, scan the repo for scrollY. accesses lacking optional chaining to prevent similar crashes elsewhere.

I can draft the unit test and open a follow-up PR to harden other call sites. To quickly scan the codebase, run:

🏁 Script executed:

#!/bin/bash
# Find potentially unsafe scrollY dereferences
rg -nP -C2 --type=ts '\bscrollY\.[A-Za-z_]\w*' -g '!**/dist/**' -g '!**/node_modules/**'

# Also check optimizeOpts-based paths
rg -nP -C2 --type=ts 'optimizeOpts(?:\.value)?\.scrollY\.[A-Za-z_]\w*' -g '!**/dist/**' -g '!**/node_modules/**'

Length of output: 2551

Guard all scrollY accesses and add a regression test

I’ve confirmed several unguarded scrollY dereferences beyond the original snippet. To prevent crashes when optimization.scrollY is null or undefined, please:

  • In packages/vue/src/grid/src/table/src/utils/computeScrollLoad.ts
    • Line 37: replace scrollY.vSize with scrollY?.vSize
    • Line 41: guard !scrollY.oSize as !scrollY?.oSize
    • Line 46: guard if (!scrollY.rSize) as if (!scrollY?.rSize)
    • (And similarly for any other direct scrollY.* usages)
  • In packages/vue/src/grid/src/table/src/methods.ts line 293
    • Change scrollY && scrollY.gt > 0 to scrollY?.gt > 0

After adding these guards (and providing sensible defaults where needed), please write a regression test that mounts the grid with:

optimization: { scrollY: null }

and asserts:

  1. No runtime error is thrown.
  2. rowHeight falls back to the theme’s default (e.g. GlobalConfig.rowHeight[tinyTheme.value]['default'] or 40).

This will catch any remaining unguarded access and prevent similar crashes in the future.

🤖 Prompt for AI Agents 
In packages/vue/src/grid/src/table/src/table.ts around lines 711-716 and in
packages/vue/src/grid/src/table/src/utils/computeScrollLoad.ts (line ~37, 41, 46
and any other direct scrollY.* usages) and
packages/vue/src/grid/src/table/src/methods.ts line 293, guard all dereferences
of optimization.scrollY with optional chaining (e.g. replace scrollY.vSize with
scrollY?.vSize, !scrollY.oSize with !scrollY?.oSize, if (!scrollY.rSize) with if
(!scrollY?.rSize), and change scrollY && scrollY.gt > 0 to scrollY?.gt > 0), and
provide sensible defaults where needed so code still falls back to GlobalConfig
rowHeight or 40; then add a regression test that mounts the grid with
optimization: { scrollY: null } and asserts no runtime error and rowHeight falls
back to the theme default.

const headerRowHeight = hooks.computed(
() => GlobalConfig.headerRowHeight[tinyTheme.value]?.[vSize.value || 'default'] || 40
Expand Down
Loading