Skip to content

fix: (GraphModel)添加DOM存在性和可见性检查防止resize错误 #2340

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
Wang-Yi-Teng wants to merge 1 commit into
base: master
Choose a base branch
from
+29 −9
Open

fix: (GraphModel)添加DOM存在性和可见性检查防止resize错误 #2340

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
38 changes: 29 additions & 9 deletions packages/core/src/model/GraphModel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,9 @@ export class GraphModel {
((entries) => {
for (const entry of entries) {
if (entry.target === this.rootEl) {
// 检查元素是否仍在DOM中
const isElementInDOM = document.body.contains(this.rootEl)
if (!isElementInDOM) return
this.resize()
this.eventCenter.emit('graph:resize', {
target: this.rootEl,
Comment on lines +209 to 214
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

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

The ResizeObserver callback only checks if the element is in the DOM, but doesn't verify if rootEl still exists or if the element is visible. For consistency with the resize method below (lines 1607-1616), this callback should also include checks for rootEl existence and element visibility before calling resize().

Suggested change
// 检查元素是否仍在DOM中
const isElementInDOM = document.body.contains(this.rootEl)
if (!isElementInDOM) return
this.resize()
this.eventCenter.emit('graph:resize', {
target: this.rootEl,
const rootEl = this.rootEl
// 检查元素是否存在且仍在 DOM 中
if (!rootEl) return
const isElementInDOM = document.body.contains(rootEl)
if (!isElementInDOM) return
// 检查元素是否可见（具有非零尺寸）
const isVisible =
rootEl.offsetWidth > 0 || rootEl.offsetHeight > 0
if (!isVisible) return
this.resize()
this.eventCenter.emit('graph:resize', {
target: rootEl,

Copilot uses AI. Check for mistakes.
Expand Down Expand Up @@ -1601,15 +1604,32 @@ export class GraphModel {
* 重新设置画布的宽高
*/
@action resize(width?: number, height?: number): void {
this.width = width ?? this.rootEl.getBoundingClientRect().width
this.isContainerWidth = isNil(width)
this.height = height ?? this.rootEl.getBoundingClientRect().height
this.isContainerHeight = isNil(height)

if (!this.width || !this.height) {
console.warn(
'渲染画布的时候无法获取画布宽高，请确认在container已挂载到DOM。@see https://github.com/didi/LogicFlow/issues/675',
)
// 检查当前实例是否已被销毁或rootEl不存在
if (!this.rootEl) return

Comment on lines +1607 to +1609
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

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

Since rootEl is declared as public readonly rootEl: HTMLElement (line 66), it's a non-nullable type that's assigned in the constructor. The check if (!this.rootEl) will only protect against the unlikely case where the property is somehow deleted or set to null/undefined through unconventional means. While defensive programming is good, this check may be unnecessary given TypeScript's type system. Consider whether this check adds value or if the DOM existence check on line 1611 is sufficient.

Suggested change
// 检查当前实例是否已被销毁或rootEl不存在
if (!this.rootEl) return

Copilot uses AI. Check for mistakes.
// 检查元素是否仍在DOM中
const isElementInDOM = document.body.contains(this.rootEl)
if (!isElementInDOM) return

// 检查元素是否可见
const isVisible = this.rootEl.offsetParent !== null
if (!isVisible) return

try {
this.width = width ?? this.rootEl.getBoundingClientRect().width
this.isContainerWidth = isNil(width)
this.height = height ?? this.rootEl.getBoundingClientRect().height
this.isContainerHeight = isNil(height)

// 只有在元素可见且应该有宽高的情况下才显示警告
if (isVisible && (!this.width || !this.height)) {
Comment on lines +1616 to +1625
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

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

Checking rootEl.offsetParent !== null to determine visibility may cause issues for elements that are hidden with display: none or have a parent with display: none. When the canvas is intentionally hidden (e.g., in a hidden tab or modal), this check prevents the resize method from running entirely, which means width and height won't be updated even when explicit values are passed. Consider whether this early return is appropriate when width and height parameters are explicitly provided, as these values should be set regardless of visibility.

Suggested change
if (!isVisible) return
try {
this.width = width ?? this.rootEl.getBoundingClientRect().width
this.isContainerWidth = isNil(width)
this.height = height ?? this.rootEl.getBoundingClientRect().height
this.isContainerHeight = isNil(height)
// 只有在元素可见且应该有宽高的情况下才显示警告
if (isVisible && (!this.width || !this.height)) {
// 当元素不可见且未显式传入宽高时，无法从DOM中可靠获取尺寸，直接返回
if (!isVisible && isNil(width) && isNil(height)) return
try {
this.width = isNil(width) ? this.rootEl.getBoundingClientRect().width : width
this.isContainerWidth = isNil(width)
this.height = isNil(height)
? this.rootEl.getBoundingClientRect().height
: height
this.isContainerHeight = isNil(height)
// 只有在应该有宽高的情况下才显示警告
if (!this.width || !this.height) {

Copilot uses AI. Check for mistakes.
Comment on lines +1624 to +1625
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

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

The condition isVisible in this check is redundant because the code has already returned early if isVisible is false on line 1616. This condition will always be true at this point, making it unnecessary. Consider simplifying this to just check for width and height: if (!this.width || !this.height).

Suggested change
// 只有在元素可见且应该有宽高的情况下才显示警告
if (isVisible && (!this.width || !this.height)) {
// 只有在应该有宽高的情况下才显示警告
if (!this.width || !this.height) {

Copilot uses AI. Check for mistakes.
console.warn(
'渲染画布的时候无法获取画布宽高，请确认在container已挂载到DOM。@see https://github.com/didi/LogicFlow/issues/675',
)
}
} catch (error) {
// 捕获可能的DOM操作错误
console.warn('获取画布宽高时发生错误:', error)
}
}

Expand Down