Skip to content

Conversation

@snomiao
Copy link
Member

@snomiao snomiao commented Aug 31, 2025

Summary

  • Fixed pnpm collect-i18n script failing with TypeScript compilation error
  • Resolved babel transformer issue with declare fields in litegraph classes
  • Isolated i18n tests to prevent CI interference

Problem

The collect-i18n script was failing with:

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

Root Cause

  • Playwright's internal TypeScript compilation doesn't properly handle declare fields in classes
  • The i18n test files were importing types that transitively import litegraph
  • litegraph contains declare fields that Playwright's babel transformer can't process

Solution

  1. Isolate i18n tests: Move i18n tests to scripts/i18n/ with separate playwright.i18n.config.ts
  2. Fix import chain: Remove direct imports of types that reference litegraph, process them in browser context
  3. Exclude from CI: Add grepInvert: /@mobile|collect-i18n/ to exclude i18n tests from regular CI runs
  4. Preserve functionality: i18n collection still works via pnpm collect-i18n when server is running

Test Plan

  • Run pnpm collect-i18n - completes without TypeScript compilation errors
  • Verify i18n tests are excluded from pnpm test:browser (0 found)
  • Check that regular browser tests still work
  • CI should now pass since i18n tests don't run during regular test suite

Changes

  • package.json: Use separate playwright config for i18n collection
  • playwright.config.ts: Exclude collect-i18n tests from regular CI runs
  • playwright.i18n.config.ts: Separate config for i18n tests
  • Move i18n tests to scripts/i18n/ directory
  • Remove problematic type imports that cause compilation errors

🤖 Generated with Claude Code

…re fields

The collect-i18n script was failing with babel transformer error when encountering TypeScript declare fields in litegraph classes.

Root cause:
- Playwright internal TypeScript compilation does not handle declare fields properly
- i18n test files imported ComfyNodeDefImpl which imports litegraph types

Solution:
- Move i18n tests to browser_tests/tests/ with .spec.ts extension
- Remove direct imports of types that reference litegraph
- Process node definitions in browser context where types are available
- Use nx e2e command for consistency with other browser tests

Fixes regression introduced after Nx integration
@github-actions
Copy link

github-actions bot commented Aug 31, 2025

🎨 Storybook Build Status

Build failed!

⏰ Completed at: 08/31/2025, 03:30:10 PM UTC

🔗 Links


⚠️ Please check the workflow logs for error details.

@github-actions
Copy link

github-actions bot commented Aug 31, 2025

🎭 Playwright Test Results

All tests passed across all browsers!

⏰ Completed at: 08/31/2025, 03:52:26 PM UTC

📊 Test Reports by Browser


🎉 Your tests are passing across all browsers!

@snomiao
Copy link
Member Author

snomiao commented Sep 26, 2025

close this as not necessary anymore because we solved locale workflow problem here: - fix(collect-i18n-node-defs): refactor to run ComfyNodeDefImpl only in browser context by snomiao · Pull Request #5775 · Comfy-Org/ComfyUI_frontend

@snomiao snomiao closed this Sep 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants