fix(collect-i18n-node-defs): refactor to run ComfyNodeDefImpl only in browser context #5775

snomiao · 2025-09-25T21:06:36Z

The Problem

The collect-i18n-node-defs.ts script started failing ~3 weeks ago when Vue nodes were introduced (commit 006e6bd57, PR #4263). The issue stems from:

Import chain bringing Vue components into Node.js context:

collect-i18n-node-defs.ts
  ↓ imports
ComfyNodeDefImpl (from nodeDefStore.ts)
  ↓ imports
useSubgraphStore (from subgraphStore.ts)
  ↓ transitively imports
executionStore.ts
  ↓ imports
ChatHistoryWidget.vue (Vue component!)

TypeScript declare fields causing Babel errors:

TypeScript 'declare' fields must first be transformed by @babel/plugin-transform-typescript

This Solution vs PR #5515

PR #5515 Approach (Complex)

Adds custom Babel plugins and configurations
Implements automatic browser globals injection
Requires 47,517 additions, 9,469 deletions
Modifies the entire Playwright babel transformation pipeline

This PR's Approach (Simple)

Uses dynamic imports to defer module loading until runtime
Avoids Babel compilation of problematic TypeScript/Vue files
Only 40 lines changed in a single file
No configuration changes needed

How This Fix Works

// Instead of static import that Babel tries to compile:
// import { ComfyNodeDefImpl } from '../src/stores/nodeDefStore'

// We use:
// 1. Type-only import (erased at runtime)
import type { ComfyNodeDefImpl } from '../src/stores/nodeDefStore'

// 2. Dynamic import at runtime (bypasses Babel)
const { ComfyNodeDefImpl: ComfyNodeDefImplClass } = await import(
  '../src/stores/nodeDefStore'
)

github-actions · 2025-09-25T21:06:56Z

🎭 Playwright Test Results

❌ Some tests failed

⏰ Completed at: 09/26/2025, 05:00:51 AM UTC

📈 Summary

Total Tests: 461
Passed: 429 ✅
Failed: 1 ❌
Flaky: 2 ⚠️
Skipped: 29 ⏭️

📊 Test Reports by Browser

✅ chromium: View Report • ✅ 422 / ❌ 1 / ⚠️ 2 / ⏭️ 29
✅ chromium-2x: View Report • ✅ 2 / ❌ 0 / ⚠️ 0 / ⏭️ 0
✅ chromium-0.5x: View Report • ✅ 1 / ❌ 0 / ⚠️ 0 / ⏭️ 0
✅ mobile-chrome: View Report • ✅ 4 / ❌ 0 / ⚠️ 0 / ⏭️ 0

🎉 Click on the links above to view detailed test results for each browser configuration.

…ection

…efs script

Myestery

Thanks a lot. I've anticipated this fix for some time now

.github/workflows/i18n.yaml

…anch detection" This reverts commit c733a27.

scripts/collect-i18n-node-defs.ts

This reverts commit e810102.

christian-byrne

LGTM!

github-actions · 2025-09-26T05:56:48Z

@snomiao Backport to core/1.27 failed: Merge conflicts detected.

Please manually cherry-pick commit 3a9365af13cfd7956bb3a69b4ebd5934e39a64be to the core/1.27 branch.

Conflicting files

scripts/collect-i18n-node-defs.ts

github-actions · 2025-09-26T05:56:50Z

@snomiao Backport to core/1.27 failed: Merge conflicts detected.

Please manually cherry-pick commit 3a9365af13cfd7956bb3a69b4ebd5934e39a64be to the core/1.27 branch.

Conflicting files

scripts/collect-i18n-node-defs.ts

christian-byrne · 2025-09-26T05:58:19Z

I will cherry pick

… browser context (#5775) The `collect-i18n-node-defs.ts` script started failing ~3 weeks ago when Vue nodes were introduced ([commit 006e6bd](006e6bd57), [PR #4263](#4263)). The issue stems from: 1. **Import chain bringing Vue components into Node.js context:** ``` collect-i18n-node-defs.ts ↓ imports ComfyNodeDefImpl (from nodeDefStore.ts) ↓ imports useSubgraphStore (from subgraphStore.ts) ↓ transitively imports executionStore.ts ↓ imports ChatHistoryWidget.vue (Vue component!) ``` 2. **TypeScript `declare` fields causing Babel errors:** ``` TypeScript 'declare' fields must first be transformed by @babel/plugin-transform-typescript ``` - Adds custom Babel plugins and configurations - Implements automatic browser globals injection - Requires **47,517 additions, 9,469 deletions** - Modifies the entire Playwright babel transformation pipeline - Uses dynamic imports to defer module loading until runtime - Avoids Babel compilation of problematic TypeScript/Vue files - **Only 40 lines changed** in a single file - No configuration changes needed ```typescript // Instead of static import that Babel tries to compile: // import { ComfyNodeDefImpl } from '../src/stores/nodeDefStore' // We use: // 1. Type-only import (erased at runtime) import type { ComfyNodeDefImpl } from '../src/stores/nodeDefStore' // 2. Dynamic import at runtime (bypasses Babel) const { ComfyNodeDefImpl: ComfyNodeDefImplClass } = await import( '../src/stores/nodeDefStore' ) ``` --------- Co-authored-by: github-actions <[email protected]>

…t fix to core/1.27 (#5796) ## Summary Backports the ComfyNodeDefImpl browser context fix from #5775 to the core/1.27 branch. ## Changes - Move ComfyNodeDefImpl instantiation to browser context to avoid Node.js compatibility issues - Add workers: 1 to playwright i18n config for consistent execution - Fix floating promise by awaiting page.route() call - Add allowDefaultProject config for playwright and script files to resolve ESLint parsing ## Original Issue The `collect-i18n-node-defs.ts` script was failing due to Vue components being imported into Node.js context, causing Babel compilation errors with TypeScript 'declare' fields. ## Solution Uses dynamic imports to defer module loading until runtime in the browser context, avoiding Babel compilation of problematic TypeScript/Vue files. Cherry-picked from 3a9365a ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5796-fix-collect-i18n-node-defs-backport-ComfyNodeDefImpl-browser-context-fix-to-core-1-27-27a6d73d365081639625f25ecf9553fd) by [Unito](https://www.unito.io) Co-authored-by: github-actions <[email protected]>

… browser context (#5775) ## The Problem The `collect-i18n-node-defs.ts` script started failing ~3 weeks ago when Vue nodes were introduced ([commit 006e6bd](006e6bd57), [PR #4263](#4263)). The issue stems from: 1. **Import chain bringing Vue components into Node.js context:** ``` collect-i18n-node-defs.ts ↓ imports ComfyNodeDefImpl (from nodeDefStore.ts) ↓ imports useSubgraphStore (from subgraphStore.ts) ↓ transitively imports executionStore.ts ↓ imports ChatHistoryWidget.vue (Vue component!) ``` 2. **TypeScript `declare` fields causing Babel errors:** ``` TypeScript 'declare' fields must first be transformed by @babel/plugin-transform-typescript ``` ## This Solution vs PR #5515 ### PR #5515 Approach (Complex) - Adds custom Babel plugins and configurations - Implements automatic browser globals injection - Requires **47,517 additions, 9,469 deletions** - Modifies the entire Playwright babel transformation pipeline ### This PR's Approach (Simple) - Uses dynamic imports to defer module loading until runtime - Avoids Babel compilation of problematic TypeScript/Vue files - **Only 40 lines changed** in a single file - No configuration changes needed ## How This Fix Works ```typescript // Instead of static import that Babel tries to compile: // import { ComfyNodeDefImpl } from '../src/stores/nodeDefStore' // We use: // 1. Type-only import (erased at runtime) import type { ComfyNodeDefImpl } from '../src/stores/nodeDefStore' // 2. Dynamic import at runtime (bypasses Babel) const { ComfyNodeDefImpl: ComfyNodeDefImplClass } = await import( '../src/stores/nodeDefStore' ) ``` --------- Co-authored-by: github-actions <[email protected]>

snomiao force-pushed the sno-fix-locale branch 4 times, most recently from 20c099f to 3016005 Compare September 26, 2025 00:14

snomiao added 2 commits September 26, 2025 09:15

Enhance locale update workflow to include 'sno-fix-locale' branch det…

c733a27

…ection

Refactor: Simplify node definitions collection in collect-i18n-node-d…

3d88e80

…efs script

snomiao force-pushed the sno-fix-locale branch from c566517 to 3d88e80 Compare September 26, 2025 00:15

snomiao marked this pull request as ready for review September 26, 2025 00:34

snomiao requested review from Yorha4D, KarryCharon, shinshin86 and a team as code owners September 26, 2025 00:34

dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Sep 26, 2025

snomiao force-pushed the sno-fix-locale branch from a237b8e to 3d88e80 Compare September 26, 2025 00:37

dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Sep 26, 2025

Update locales [skip ci]

e810102

dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Sep 26, 2025

snomiao requested review from DrJKL and christian-byrne September 26, 2025 01:01

Myestery reviewed Sep 26, 2025

View reviewed changes

.github/workflows/i18n.yaml Outdated Show resolved Hide resolved

Revert "Enhance locale update workflow to include 'sno-fix-locale' br…

84b5b45

…anch detection" This reverts commit c733a27.

Myestery previously approved these changes Sep 26, 2025

View reviewed changes

Merge branch 'main' into sno-fix-locale

6c5cb8b

snomiao enabled auto-merge (squash) September 26, 2025 04:28

snomiao mentioned this pull request Sep 26, 2025

fix: Add babel configuration for Playwright to handle TypeScript declare fields #5515

Closed

DrJKL requested changes Sep 26, 2025

View reviewed changes

scripts/collect-i18n-node-defs.ts Outdated Show resolved Hide resolved

christian-byrne added the area:i18n Anything related to translation label Sep 26, 2025

christian-byrne added the area:testing label Sep 26, 2025

snomiao disabled auto-merge September 26, 2025 04:52

snomiao added 2 commits September 26, 2025 05:00

fix: use ComfyNodeDef type for node definitions filtering and mapping

3bc739a

Revert "Update locales [skip ci]"

0ad76b9

This reverts commit e810102.

snomiao dismissed Myestery’s stale review via 0ad76b9 September 26, 2025 05:03

dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Sep 26, 2025

snomiao enabled auto-merge (squash) September 26, 2025 05:04

christian-byrne approved these changes Sep 26, 2025

View reviewed changes

christian-byrne disabled auto-merge September 26, 2025 05:56

christian-byrne merged commit 3a9365a into main Sep 26, 2025
3 checks passed

christian-byrne deleted the sno-fix-locale branch September 26, 2025 05:56

christian-byrne added needs-backport Fix/change that needs to be cherry-picked to the current feature freeze branch 1.27 labels Sep 26, 2025

This was referenced Sep 26, 2025

refactor: Remove TypeScript declare keywords for Playwright compatibility #5304

Closed

[fix] Fix collect-i18n script compilation error with TypeScript declare fields #5287

Closed

refactor: Improve litegraph preprocessing for i18n collection #5297

Closed

christian-byrne mentioned this pull request Sep 26, 2025

fix(collect-i18n-node-defs): backport ComfyNodeDefImpl browser context fix to core/1.27 #5796

Merged

snomiao mentioned this pull request Sep 30, 2025

[refactor] Solve circular dependency in LiteGraph module #5604

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(collect-i18n-node-defs): refactor to run ComfyNodeDefImpl only in browser context #5775

fix(collect-i18n-node-defs): refactor to run ComfyNodeDefImpl only in browser context #5775

Uh oh!

snomiao commented Sep 25, 2025 •

edited by christian-byrne

Loading

Uh oh!

github-actions bot commented Sep 25, 2025 •

edited

Loading

Uh oh!

Myestery left a comment

Uh oh!

Uh oh!

Uh oh!

christian-byrne left a comment

Uh oh!

Uh oh!

github-actions bot commented Sep 26, 2025

Uh oh!

github-actions bot commented Sep 26, 2025

Uh oh!

christian-byrne commented Sep 26, 2025

Uh oh!

Uh oh!

fix(collect-i18n-node-defs): refactor to run ComfyNodeDefImpl only in browser context #5775

fix(collect-i18n-node-defs): refactor to run ComfyNodeDefImpl only in browser context #5775

Uh oh!

Conversation

snomiao commented Sep 25, 2025 • edited by christian-byrne Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The Problem

This Solution vs PR #5515

PR #5515 Approach (Complex)

This PR's Approach (Simple)

How This Fix Works

Uh oh!

github-actions bot commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎭 Playwright Test Results

📈 Summary

📊 Test Reports by Browser

Uh oh!

Myestery left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

christian-byrne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Sep 26, 2025

Uh oh!

github-actions bot commented Sep 26, 2025

Uh oh!

christian-byrne commented Sep 26, 2025

Uh oh!

Uh oh!

snomiao commented Sep 25, 2025 •

edited by christian-byrne

Loading

github-actions bot commented Sep 25, 2025 •

edited

Loading