Skip to content

Conversation

@christian-byrne
Copy link
Contributor

@christian-byrne christian-byrne commented Sep 26, 2025

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 by Unito

… 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]>
@christian-byrne christian-byrne requested a review from a team as a code owner September 26, 2025 06:18
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Sep 26, 2025
@github-actions
Copy link

github-actions bot commented Sep 26, 2025

🎭 Playwright Test Results

⚠️ Tests passed with flaky tests

⏰ Completed at: 09/26/2025, 06:29:52 AM UTC

📈 Summary

  • Total Tests: 447
  • Passed: 415 ✅
  • Failed: 0
  • Flaky: 3 ⚠️
  • Skipped: 29 ⏭️

📊 Test Reports by Browser

  • chromium: View Report • ✅ 408 / ❌ 0 / ⚠️ 3 / ⏭️ 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.

@christian-byrne christian-byrne added the backport Backporting a PR onto a release candidate label Sep 26, 2025
@AustinMroz AustinMroz merged commit a919483 into core/1.27 Sep 26, 2025
25 checks passed
@AustinMroz AustinMroz deleted the backport-collect-i18n-fix-core-1.27 branch September 26, 2025 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport Backporting a PR onto a release candidate size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants