[Bug] Node preview images are lost when switching between multiple workflow tabs#9380
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe ChangeTracker.store() method now copies Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
🎭 Playwright: ✅ 13 passed, 0 failed📊 Browser Reports
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/scripts/changeTracker.ts`:
- Line 80: The current assignment this.nodeOutputs = { ...app.nodeOutputs } only
shallow-copies the map so nested objects and images arrays remain shared and can
be mutated after store(); replace this with a deep clone when snapshotting
nodeOutputs (e.g., use structuredClone(app.nodeOutputs) or a deepClone/cloneDeep
utility) so that ChangeTracker's nodeOutputs is fully immutable after
assignment; update the code paths that create the snapshot (the nodeOutputs
assignment in changeTracker.ts and any related store() snapshot logic) to use
the chosen deep-clone method to ensure nested objects/arrays are copied, not
referenced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5229a8d8-c006-4e85-96bd-94c839323fa8
📒 Files selected for processing (1)
src/scripts/changeTracker.ts
📦 Bundle: 4.48 MB gzip 🔴 +50 BDetailsSummary
Category Glance App Entry Points — 17.7 kB (baseline 17.7 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 912 kB (baseline 912 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 72.1 kB (baseline 72.1 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 436 kB (baseline 436 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 736 B (baseline 736 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 47 kB (baseline 47 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 2.73 MB (baseline 2.73 MB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 13 added / 13 removed Utilities & Hooks — 55.5 kB (baseline 55.5 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 11 added / 11 removed Vendor & Third-Party — 8.86 MB (baseline 8.86 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.89 MB (baseline 7.89 MB) • 🔴 +45 BBundles that do not match a named category
Status: 51 added / 51 removed |
…puts snapshot (#9387) ## Summary Follow-up to #9380. Replaces local `clone()` with shared util and centralizes output snapshotting. ## Changes - **What**: Replaced local `JSON.parse(JSON.stringify)` clone in `changeTracker.ts` with shared `clone()` from `@/scripts/utils` (prefers `structuredClone` with JSON fallback). Added `snapshotOutputs()` to `useNodeOutputStore` as symmetric counterpart to existing `restoreOutputs()`, and wired `changeTracker.store()` to use it. - **Breaking**: None ## Review Focus Symmetry between `snapshotOutputs()` and `restoreOutputs()` in the node output store. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9387-refactor-changeTracker-use-shared-clone-util-and-centralize-nodeOutputs-snapshot-3196d73d365081a289c3cb414f57929e) by [Unito](https://www.unito.io)
…rkflow tabs (#9380) ## Summary When working with multiple workflow tabs, the internal preview (image thumbnail) of nodes like Load Image disappears after navigating away from and back to a tab. This affects all active tabs once the switch occurs. ## Screenshot before https://github.com/user-attachments/assets/99466123-37db-406f-9e17-0a9ff22311c3 after https://github.com/user-attachments/assets/bdad0ef1-72b7-46c7-aa61-0a557688e55e --------- Co-authored-by: Alexander Brown <drjkl@comfy.org>
…puts snapshot (#9387) ## Summary Follow-up to #9380. Replaces local `clone()` with shared util and centralizes output snapshotting. ## Changes - **What**: Replaced local `JSON.parse(JSON.stringify)` clone in `changeTracker.ts` with shared `clone()` from `@/scripts/utils` (prefers `structuredClone` with JSON fallback). Added `snapshotOutputs()` to `useNodeOutputStore` as symmetric counterpart to existing `restoreOutputs()`, and wired `changeTracker.store()` to use it. - **Breaking**: None ## Review Focus Symmetry between `snapshotOutputs()` and `restoreOutputs()` in the node output store. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9387-refactor-changeTracker-use-shared-clone-util-and-centralize-nodeOutputs-snapshot-3196d73d365081a289c3cb414f57929e) by [Unito](https://www.unito.io)
…rkflow tabs (#9380) ## Summary When working with multiple workflow tabs, the internal preview (image thumbnail) of nodes like Load Image disappears after navigating away from and back to a tab. This affects all active tabs once the switch occurs. ## Screenshot before https://github.com/user-attachments/assets/99466123-37db-406f-9e17-0a9ff22311c3 after https://github.com/user-attachments/assets/bdad0ef1-72b7-46c7-aa61-0a557688e55e --------- Co-authored-by: Alexander Brown <drjkl@comfy.org>
|
@kaili-yang Successfully backported to #9571 |
…g between multiple workflow tabs (#9571) Backport of #9380 to `core/1.40` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9571-backport-core-1-40-Bug-Node-preview-images-are-lost-when-switching-between-multiple-w-31d6d73d36508164b4e3d90b756f51fa) by [Unito](https://www.unito.io) Co-authored-by: Kelly Yang <124ykl@gmail.com> Co-authored-by: Alexander Brown <drjkl@comfy.org>
Coverage for 10 bug gaps identified during deep persistence audit: CRITICAL: - PR #9531: Workflow data corruption from checkState during graph loading (pythongosssss fix — had ZERO tests, now covered with 2 tests) - PR #9533: Desynced workflow/graph state during rapid tab switching MEDIUM: - PR #9380: Node output previews lost on tab switch - 44bb6f1: Canvas not cleared before loading new workflow - PR #7648: Widget values lost on graph change - PR #9694: API format workflows fail with missing nodes - PR #8259: Middle-click paste duplicates workflow - PR #8715: Transient blob: URLs in serialization LOW: - PR #8963: Locale change breaks workflows - Splitter panel size drift All tests use Vue nodes with new menu enabled. Each test documents which PR/commit it regresses and reproduces the exact user scenario. Part of: Test Coverage Q2 Overhaul (REG-01)

Summary
When working with multiple workflow tabs, the internal preview (image thumbnail) of nodes like Load Image disappears after navigating away from and back to a tab. This affects all active tabs once the switch occurs.
Screenshot
before
before-image-loss.mp4
after
after-iamge-loss.mp4