Skip to content

fix(components): [table] expanded rows cannot be updated using array methods#23761

Merged
keeplearning66 merged 3 commits into
devfrom
fix/23759-case-1
Mar 12, 2026
Merged

fix(components): [table] expanded rows cannot be updated using array methods#23761
keeplearning66 merged 3 commits into
devfrom
fix/23759-case-1

Conversation

@rzzf
Copy link
Copy Markdown
Member

@rzzf rzzf commented Mar 10, 2026

Fix the first case of #23759

demo

Summary by CodeRabbit

  • Bug Fixes

    • Improved table row expansion reactivity so nested or runtime changes to expanded rows are reliably detected and reflected in the UI.
  • Tests

    • Added a runtime test that mutates expanded rows and verifies dynamic show/hide behavior to prevent regressions.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 10, 2026

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 10, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b5c7d771-a756-4039-9c2b-389ad51f9fd1

📥 Commits

Reviewing files that changed from the base of the PR and between 19fbe19 and 91f32ec.

📒 Files selected for processing (1)
  • packages/components/table/__tests__/table.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/components/table/tests/table.test.ts

📝 Walkthrough

Walkthrough

Changed the table tree store watcher to use deep watching for expandRowKeys.value and added a test that mutates expandRowKeys at runtime to verify visibility updates after array mutations.

Changes

Cohort / File(s) Summary
Watcher Configuration
packages/components/table/src/store/tree.ts
Enabled deep: true on the expandRowKeys.value watcher so nested mutations within the expanded keys array trigger the watcher.
Tests
packages/components/table/__tests__/table.test.ts
Added runtime mutations (splice/push) to expandRowKeys in a tree-expand test, awaiting updates and asserting level-1 row visibility after each mutation. Review for timing/flakiness.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

Contribution::Community, Needs Review

Suggested reviewers

  • btea

Poem

🐰 I nudge the keys with a curious paw,
I watch the branches shift and thaw,
I splice, I push, the rows rearrange—
Tiny hops make the view exchange. 🥕✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description references the issue #23759, provides a demo link, and specifies which case is being fixed. However, it lacks explicit coverage of the required checklist items in the template. While substantive content is present, explicitly confirm you followed the contributing guide, are merging to 'dev', and check the template checklist boxes for clarity.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title directly addresses the main change: fixing expanded rows not updating when using array methods like splice/push on expandRowKeys.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/23759-case-1

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the CommitMessage::Unqualified Unqualified commit message label Mar 10, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 10, 2026

Hello, @rzzf, your PR title does not meet the standards, please refer to here.
你好,@rzzf,你的 PR 标题不符合规范,请参考这里

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Mar 10, 2026

Open in StackBlitz

pnpm add https://pkg.pr.new/element-plus@23761
npm i https://pkg.pr.new/element-plus@23761
yarn add https://pkg.pr.new/[email protected]

commit: 7c523c1

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 10, 2026

Size Change: +27 B (0%)

Total Size: 1.45 MB

Filename Size Change
./dist/element-plus/dist/index.full.js 427 kB +12 B (0%)
./dist/element-plus/dist/index.full.min.js 283 kB +4 B (0%)
./dist/element-plus/dist/index.full.min.mjs 276 kB +2 B (0%)
./dist/element-plus/dist/index.full.mjs 418 kB +9 B (0%)
ℹ️ View Unchanged
Filename Size
./dist/element-plus/dist/index.css 46.8 kB

compressed-size-action

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 10, 2026

🧪 Playground Preview: https://element-plus.run/?pr=23761
Please comment the example via this playground if needed.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 10, 2026

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 85.49% 18067 / 21132
🔵 Statements 84.28% 18914 / 22441
🔵 Functions 83.4% 4831 / 5792
🔵 Branches 73.97% 10437 / 14108
File Coverage
File Stmts % Branch % Funcs % Lines Uncovered Lines
Changed Files
packages/components/table/src/store/tree.ts 94.16% 71.76% 100% 95.48% 41, 95, 126, 130-138, 211, 226-227
Generated in workflow #1779

@btea
Copy link
Copy Markdown
Member

btea commented Mar 10, 2026

Should we add a test case?

@rzzf
Copy link
Copy Markdown
Member Author

rzzf commented Mar 10, 2026

done

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
packages/components/table/__tests__/table.test.ts (1)

2060-2070: Add an in-place append case here too.

This only verifies the splice/removal path. Since the regression is about array-method updates to expandRowKeys, it would be good to also assert that push expands newly added keys without replacing the whole array.

Possible follow-up
       wrapper.vm.expandRowKeys.splice(1, 1)
       await doubleWait()
       childRows = wrapper.findAll('.el-table__row--level-1')
       childRows.forEach((item, index) => {
         if (index < 2) {
           expect(item.attributes('style')).toBe('')
         } else {
           expect(item.attributes('style')).toContain('display: none')
         }
       })
+
+      wrapper.vm.expandRowKeys.push('1999-3-31')
+      await doubleWait()
+      childRows = wrapper.findAll('.el-table__row--level-1')
+      childRows.forEach((item) => {
+        expect(item.attributes('style') ?? '').not.toContain('display: none')
+      })
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/components/table/__tests__/table.test.ts` around lines 2060 - 2070,
Test only covered splice/removal of wrapper.vm.expandRowKeys; add an in-place
append case that uses wrapper.vm.expandRowKeys.push(...) followed by await
doubleWait() and then re-evaluate childRows =
wrapper.findAll('.el-table__row--level-1') to assert that pushed keys cause the
corresponding rows to be visible (style not containing 'display: none') while
existing expanded rows remain visible (i.e., push should expand new rows without
replacing the array). Update the test near the existing splice block to perform
a push of a key that was previously hidden, await doubleWait(), and add
expectations for both previously-expanded and newly-expanded rows using the same
attribute checks as the splice assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/components/table/__tests__/table.test.ts`:
- Around line 2060-2070: Test only covered splice/removal of
wrapper.vm.expandRowKeys; add an in-place append case that uses
wrapper.vm.expandRowKeys.push(...) followed by await doubleWait() and then
re-evaluate childRows = wrapper.findAll('.el-table__row--level-1') to assert
that pushed keys cause the corresponding rows to be visible (style not
containing 'display: none') while existing expanded rows remain visible (i.e.,
push should expand new rows without replacing the array). Update the test near
the existing splice block to perform a push of a key that was previously hidden,
await doubleWait(), and add expectations for both previously-expanded and
newly-expanded rows using the same attribute checks as the splice assertions.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 792f66df-c4d6-40ed-8061-53d96dc5067c

📥 Commits

Reviewing files that changed from the base of the PR and between 93cda73 and 19fbe19.

📒 Files selected for processing (1)
  • packages/components/table/__tests__/table.test.ts

Copy link
Copy Markdown
Member

@keeplearning66 keeplearning66 left a comment

Choose a reason for hiding this comment

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

👍

@keeplearning66 keeplearning66 merged commit 341a08c into dev Mar 12, 2026
16 checks passed
@keeplearning66 keeplearning66 deleted the fix/23759-case-1 branch March 12, 2026 01:03
@github-actions
Copy link
Copy Markdown
Contributor

@rzzf Thanks for your contribution! ❤️

@element-bot element-bot mentioned this pull request Mar 20, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CommitMessage::Unqualified Unqualified commit message

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants