fix: ensure child node is removed from parent by thjxs · Pull Request #5011 · antvis/X6

thjxs · 2026-01-22T12:13:43Z

📝 Description

collection 的 removed 事件回调中存在将 cell.model 置空的逻辑，且该事件在调用 cell.remove 之前触发，导致删除流程异常，具体表现为：

移除子节点后，父节点的 store 仍然保留子节点 ID；
移除父节点时，子节点未被一并删除。

本次修改新增 afterRemoved 事件，并将 cell.model 置空的逻辑放在该事件中处理。

🖼️ Screenshot

console.log('before remove children: ', parent.toJSON().children)
graph.removeCells([child1])
console.log('after remove children: ', parent.toJSON().children)

Before

After

💡 Motivation and Context

🧩 Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Enhancement (changes that improvement of current feature or performance)
Refactoring (changes that neither fixes a bug nor adds a feature)
Test Case (changes that add missing tests or correct existing tests)
Code style optimization (changes that do not affect the meaning of the code)
Docs (changes that only update documentation)
Chore (changes that don't modify src or test files)

🔍 Self Check before Merge

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

gemini-code-assist · 2026-01-22T12:13:59Z

Summary of Changes

Hello @thjxs, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

此拉取请求旨在解决 cell.remove 方法中存在的父子节点关联关系处理不当的缺陷。此前，在子节点被移除后，父节点的内部存储可能仍然保留着对该子节点的引用，导致数据不一致。通过重新调整 unreference 的调用顺序并引入显式的父节点子节点移除逻辑，确保了在子节点移除时，父节点能够正确地解除与子节点的关联。

Highlights

修复子节点移除后父节点仍保留子节点引用问题: 调整了 unreference 方法的调用时机，并在移除子节点时显式调用 parent.removeChild(cell)，以确保父子节点关联关系被正确清除。
测试用例增强: 为 FakeCell 类添加了 parent 属性和 getParent() 方法，以支持在测试中模拟和验证父子节点关系。

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request fixes a bug where a child node is not correctly removed from its parent's data store. The change correctly adjusts the order of operations during cell removal, ensuring the parent-child relationship is updated before the cell is unreferenced. The fix is logical and effectively addresses the issue. The code looks good.

src/model/collection.ts

GaoFuhong · 2026-01-27T09:02:07Z

src/model/collection.ts


      if (!options.dryrun) {
+        if (parent) {
+          parent.removeChild(cell)


且这里 parent.removeChild(cell) 没有把 options 传进去，如果用户 removeCells({ silent: true }) 会出错。

src/model/collection.ts

thjxs · 2026-01-30T16:09:08Z

done

thjxs · 2026-01-30T18:13:23Z

src/model/model.ts

    }
-
-    if (cell.model === this) {
-      cell.model = null


重新梳理了一下删除逻辑，发现父子节点的删除过程中由于移除了 model 的引用，导致父子节点管理关系异常，甚至出现删除父节点但子节点未被删除的情况，所以增加了一个 afterRemoved 事件来处理这些清理工作。

safe1ine · 2026-02-05T06:35:30Z

@MonkeyCode-AI review 一下

MonkeyCode-AI · 2026-02-05T06:35:57Z

MonkeyCode-AI 正在分析任务...

MonkeyCode-AI

我是 MonkeyCode AI 编程助手，你可以在 GitHub 仓库的 PR 中 at @MonkeyCode-AI 来呼唤我。

任务执行细节请参考: https://monkeycode-ai.com

代码审查结果

通过新增 afterRemoved 事件并延后清理 cell.model，修复了 collection removed 回调过早置空导致的父子关系残留问题，但目前事件触发时机与静默/ dryrun 语义仍有改进空间，且测试覆盖不足。

🚨 Critical	⚠️ Warning	💡 Suggestion
0	2	2

MonkeyCode-AI · 2026-02-05T06:37:02Z

src/model/collection.ts

+    for (let i = 0; i < removed.length; i += 1) {
+      const cell = removed[i]
+      this.trigger('afterRemoved', { cell, options })
+    }


Warning
⚠️ afterRemoved 在 silent: true 时仍会触发，可能违反既有事件语义

当前 removed 事件会受 options.silent 控制，但新增的 afterRemoved 循环不受 silent 控制：即使调用方显式 silent: true，仍会触发 afterRemoved。这会让新增事件的语义与其它事件不一致，并可能导致依赖 silent 的调用方出现副作用（例如批量/重置/内部清理流程中不希望触发任何事件）。尤其是本 PR 在 Model.setup 中订阅了 afterRemoved 来做 cell.model=null，这意味着即便 silent 删除也会改变 cell 状态。

建议: 将 afterRemoved 的触发也置于 !options.silent 条件下；或者至少在事件命名/文档中明确它是一个“内部生命周期钩子”，不受 silent 影响（但这通常更难维护）。更推荐与 removed 保持一致。

Suggested change

for (let i = 0; i < removed.length; i += 1) {

const cell = removed[i]

this.trigger('afterRemoved', { cell, options })

}

for (let i = 0; i < removed.length; i += 1) {

const cell = removed[i]

if (!options.silent) {

this.trigger('afterRemoved', { cell, options })

}

}

MonkeyCode-AI · 2026-02-05T06:37:02Z

src/model/collection.ts

+    for (let i = 0; i < removed.length; i += 1) {
+      const cell = removed[i]
+      this.trigger('afterRemoved', { cell, options })
+    }


Warning
⚠️ afterRemoved 在 dryrun: true 时也会触发，可能引入额外副作用

dryrun 的语义通常是“不执行实际移除动作/副作用”，当前实现虽然不会调用 cell.remove()，但仍会触发 removed（在非 silent 情况下）以及 afterRemoved。这会导致 dryrun 也可能把 cell.model 置空（Model 订阅了 afterRemoved），与 dryrun 预期相悖，进而影响调用方用 dryrun 做探测/预演的场景。

建议: 让 afterRemoved 与真正的移除副作用保持一致：仅在 !options.dryrun 时触发（或至少在 Model 的处理逻辑中判断 !args.options.dryrun）。

Suggested change

for (let i = 0; i < removed.length; i += 1) {

const cell = removed[i]

this.trigger('afterRemoved', { cell, options })

}

for (let i = 0; i < removed.length; i += 1) {

const cell = removed[i]

if (!options.dryrun) {

this.trigger('afterRemoved', { cell, options })

}

}

MonkeyCode-AI · 2026-02-05T06:37:02Z

src/model/model.ts

+    collection.on('afterRemoved', (args) => {
+      const cell = args.cell
+      if (cell.model === this) {
+        cell.model = null
+      }
+    })


Tip
💡 建议在 afterRemoved 回调中同时检查 dryrun，避免预演删除修改 cell 状态

即便 collection 侧不调整事件触发条件，Model 这里也可以增加一层保护，保证 dryrun 不会改变 cell.model（属于更安全的防御式编程）。当前实现会在 dryrun 时把 cell.model 置空，可能影响外部逻辑（例如调用方 dryrun 后继续使用 cell 并假设仍挂在 model 上）。

建议: 在回调中增加 !args.options.dryrun 判断。

Suggested change

collection.on('afterRemoved', (args) => {

const cell = args.cell

if (cell.model === this) {

cell.model = null

}

})

collection.on('afterRemoved', (args) => {

const cell = args.cell

if (!args.options.dryrun && cell.model === this) {

cell.model = null

}

})

MonkeyCode-AI · 2026-02-05T06:37:02Z

__tests__/model/collection.spec.ts

+  private parent?: FakeCell
+
  constructor(
    id: string,
    opts: { zIndex?: number; type?: 'node' | 'edge' | null } = {},
+    parent?: FakeCell
  ) {
    this.id = id
    this.zIndex = opts.zIndex ?? 0
    this.type = opts.type ?? null
    this.props = {}
    this.listeners = {}
+    this.parent = parent
+  }
+
+  getParent() {
+    if (this.parent) {
+      return this.parent
+    }
+    return null


Tip
💡 测试用 FakeCell 新增 parent/getParent 但未新增对应断言用例，未覆盖该 PR 的核心场景

本次修复的核心是“删除子节点后父节点 children/store 不应残留，以及删除父节点应级联删除子节点”。但当前测试文件仅新增了 parent/getParent，并未新增关于删除父子关系的一条回归用例断言（且 FakeCell.toJSON 也不包含 children 信息）。这会导致 PR 描述中的 bug 没有自动化覆盖，后续容易回归。

建议: 补充一条最小回归测试：构造 parent/child 关系（需要 FakeCell 至少能表达 children 或能模拟 store 行为），执行 remove(child) 后断言 parent 的 children 变更；执行 remove(parent) 后断言 child.remove 被调用/child 被从 collection 移除。若测试难以在 Collection 层完成，可在更接近真实 Cell/Model 的测试层补充。

Suggested change

private parent?: FakeCell

constructor(

id: string,

opts: { zIndex?: number; type?: 'node' | 'edge' | null } = {},

parent?: FakeCell

) {

this.id = id

this.zIndex = opts.zIndex ?? 0

this.type = opts.type ?? null

this.props = {}

this.listeners = {}

this.parent = parent

}

getParent() {

if (this.parent) {

return this.parent

}

return null

gemini-code-assist bot reviewed Jan 22, 2026

View reviewed changes

GaoFuhong reviewed Jan 27, 2026

View reviewed changes

thjxs closed this Jan 30, 2026

thjxs force-pushed the master branch from ad38213 to a243837 Compare January 30, 2026 15:58

fix: ensure child node is removed from parent

ff915a9

thjxs reopened this Jan 30, 2026

refactor(model): move cell.model cleanup to afterRemoved event

2b1af53

thjxs force-pushed the master branch from c98caa0 to 2b1af53 Compare January 30, 2026 18:02

thjxs commented Jan 30, 2026

View reviewed changes

thjxs requested a review from GaoFuhong January 31, 2026 04:49

MonkeyCode-AI reviewed Feb 5, 2026

View reviewed changes

wip

d2cbd59

Conversation

thjxs commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Description

🖼️ Screenshot

💡 Motivation and Context

🧩 Types of changes

🔍 Self Check before Merge

Uh oh!

gemini-code-assist bot commented Jan 22, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

GaoFuhong Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thjxs commented Jan 30, 2026

Uh oh!

thjxs Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

safe1ine commented Feb 5, 2026

Uh oh!

MonkeyCode-AI commented Feb 5, 2026

Uh oh!

MonkeyCode-AI left a comment

Choose a reason for hiding this comment

代码审查结果

Uh oh!

MonkeyCode-AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

MonkeyCode-AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

MonkeyCode-AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

MonkeyCode-AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

thjxs commented Jan 22, 2026 •

edited

Loading