mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 14:30:41 +00:00
fix: address agent review feedback
- Fix memory leak: remove element from deferredElements on unmount - Add nextFrame() after repositionNodes in Vue mode E2E tests - Guard against zero-size collapsed measurements - Wrap getCollapsedSize callback in try-catch for Y.Map resilience - Improve measure() cache test to verify output values after invalidation - Add layoutStore collapsed size lifecycle unit tests - Fix LGraphNode.test.ts for updated useVueElementTracking signature - Use @e2e/ path alias for E2E test imports
This commit is contained in:
@@ -1,9 +1,9 @@
|
||||
import { expect } from '@playwright/test'
|
||||
import type { Page } from '@playwright/test'
|
||||
|
||||
import type { ComfyPage } from '../fixtures/ComfyPage'
|
||||
import { comfyPageFixture as test } from '../fixtures/ComfyPage'
|
||||
import { measureSelectionBounds } from '../fixtures/helpers/boundsUtils'
|
||||
import type { ComfyPage } from '@e2e/fixtures/ComfyPage'
|
||||
import { comfyPageFixture as test } from '@e2e/fixtures/ComfyPage'
|
||||
import { measureSelectionBounds } from '@e2e/fixtures/helpers/boundsUtils'
|
||||
|
||||
const SUBGRAPH_ID = '2'
|
||||
const REGULAR_ID = '3'
|
||||
@@ -87,6 +87,7 @@ test.describe(
|
||||
[refId]: REF_POS,
|
||||
[targetId]: TARGET_POSITIONS[pos]
|
||||
})
|
||||
await comfyPage.nextFrame()
|
||||
await comfyPage.vueNodes.waitForNodes()
|
||||
await comfyPage.vueNodes.getNodeLocator(targetId).waitFor()
|
||||
await comfyPage.vueNodes.getNodeLocator(refId).waitFor()
|
||||
|
||||
@@ -78,7 +78,13 @@ function useVueNodeLifecycleIndividual() {
|
||||
shouldRenderVueNodes,
|
||||
() => {
|
||||
LiteGraph.getCollapsedSize = shouldRenderVueNodes.value
|
||||
? (nodeId) => layoutStore.getNodeCollapsedSize(String(nodeId))
|
||||
? (nodeId) => {
|
||||
try {
|
||||
return layoutStore.getNodeCollapsedSize(String(nodeId))
|
||||
} catch {
|
||||
return undefined
|
||||
}
|
||||
}
|
||||
: undefined
|
||||
},
|
||||
{ immediate: true }
|
||||
|
||||
@@ -705,14 +705,20 @@ describe('LGraphNode', () => {
|
||||
LiteGraph.getCollapsedSize = spy
|
||||
|
||||
node.measure(out)
|
||||
expect(spy).toHaveBeenCalledTimes(1)
|
||||
expect(out[2]).toBe(180)
|
||||
expect(out[3]).toBe(36)
|
||||
|
||||
node.flags.collapsed = false
|
||||
node.measure(out)
|
||||
expect(out[2]).toBe(node.size[0])
|
||||
|
||||
node.flags.collapsed = true
|
||||
const newSize = { width: 200, height: 40 }
|
||||
spy.mockReturnValue(newSize)
|
||||
node.measure(out)
|
||||
expect(spy).toHaveBeenCalledTimes(2)
|
||||
expect(out[2]).toBe(200)
|
||||
expect(out[3]).toBe(40)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -645,4 +645,42 @@ describe('layoutStore CRDT operations', () => {
|
||||
expect(layoutStore.getSlotLayout(slotKey)).toEqual(slotLayout)
|
||||
}
|
||||
)
|
||||
|
||||
describe('collapsed size lifecycle', () => {
|
||||
const nodeId = 'collapsed-node'
|
||||
|
||||
beforeEach(() => {
|
||||
layoutStore.initializeFromLiteGraph([
|
||||
{ id: nodeId, pos: [0, 0], size: [200, 100] }
|
||||
])
|
||||
})
|
||||
|
||||
it('stores and retrieves collapsed size', () => {
|
||||
layoutStore.updateNodeCollapsedSize(nodeId, { width: 180, height: 36 })
|
||||
expect(layoutStore.getNodeCollapsedSize(nodeId)).toEqual({
|
||||
width: 180,
|
||||
height: 36
|
||||
})
|
||||
})
|
||||
|
||||
it('clears collapsed size', () => {
|
||||
layoutStore.updateNodeCollapsedSize(nodeId, { width: 180, height: 36 })
|
||||
layoutStore.clearNodeCollapsedSize(nodeId)
|
||||
expect(layoutStore.getNodeCollapsedSize(nodeId)).toBeUndefined()
|
||||
})
|
||||
|
||||
it('returns undefined for non-existent node', () => {
|
||||
expect(layoutStore.getNodeCollapsedSize('no-such-node')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('returns undefined when ynode has invalid collapsedSize data', () => {
|
||||
// Write a non-Size value directly to simulate corrupted CRDT data
|
||||
layoutStore.updateNodeCollapsedSize(nodeId, { width: 10, height: 20 })
|
||||
// Valid data should work
|
||||
expect(layoutStore.getNodeCollapsedSize(nodeId)).toEqual({
|
||||
width: 10,
|
||||
height: 20
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { createTestingPinia } from '@pinia/testing'
|
||||
import { render, screen } from '@testing-library/vue'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { computed, toValue } from 'vue'
|
||||
import { computed } from 'vue'
|
||||
import type { ComponentProps } from 'vue-component-type-helpers'
|
||||
import { createI18n } from 'vue-i18n'
|
||||
|
||||
@@ -176,13 +176,7 @@ describe('LGraphNode', () => {
|
||||
it('should call resize tracking composable with node ID', () => {
|
||||
renderLGraphNode({ nodeData: mockNodeData })
|
||||
|
||||
expect(useVueElementTracking).toHaveBeenCalledWith(
|
||||
expect.any(Function),
|
||||
'node'
|
||||
)
|
||||
const idArg = vi.mocked(useVueElementTracking).mock.calls[0]?.[0]
|
||||
const id = toValue(idArg)
|
||||
expect(id).toEqual('test-node-123')
|
||||
expect(useVueElementTracking).toHaveBeenCalledWith('test-node-123', 'node')
|
||||
})
|
||||
|
||||
it('should render with data-node-id attribute', () => {
|
||||
|
||||
@@ -148,6 +148,7 @@ const resizeObserver = new ResizeObserver((entries) => {
|
||||
body instanceof HTMLElement ? body.offsetWidth : element.offsetWidth
|
||||
const collapsedHeight = element.offsetHeight
|
||||
const collapsedSize = { width: collapsedWidth, height: collapsedHeight }
|
||||
if (collapsedWidth === 0 && collapsedHeight === 0) continue
|
||||
const nodeLayout = layoutStore.getNodeLayoutRef(nodeId).value
|
||||
if (nodeLayout) {
|
||||
layoutStore.updateNodeCollapsedSize(nodeId, collapsedSize)
|
||||
@@ -318,6 +319,7 @@ export function useVueElementTracking(
|
||||
delete element.dataset[config.dataAttribute]
|
||||
cachedNodeMeasurements.delete(element)
|
||||
elementsNeedingFreshMeasurement.delete(element)
|
||||
deferredElements.delete(element)
|
||||
resizeObserver.unobserve(element)
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user