-
Notifications
You must be signed in to change notification settings - Fork 429
fix: normalize path separators in comfyAPIPlugin for Windows compatibility #7087
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/02/2025, 02:03:18 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 12/02/2025, 02:12:50 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
📝 WalkthroughWalkthroughPath normalization fix in the ComfyUI API plugin. The relative path normalization now applies earlier in the pipeline to use forward slashes consistently. The fileKey calculation was adjusted to remove trailing slash normalization, preserving backslashes for the key transformation on Windows systems. Changes
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
build/plugins/comfyAPIPlugin.ts (1)
98-100: DerivingfileKeyfrom the normalizedrelativePathcorrectly fixes Windows‑only warning mismatchesUsing
relativePath.replace(/\.ts$/, '')forfileKeynow aligns the key with the forward‑slash entries inSKIP_WARNING_FILESandDEPRECATED_FILES, so skip/deprecation checks behave consistently on Windows and non‑Windows. If this pattern is reused elsewhere later, consider extracting a small helper liketoWarningKey(id)to keep the normalization logic in one place.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
build/plugins/comfyAPIPlugin.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
build/plugins/comfyAPIPlugin.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
build/plugins/comfyAPIPlugin.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
build/plugins/comfyAPIPlugin.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
build/plugins/comfyAPIPlugin.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
build/plugins/comfyAPIPlugin.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
build/plugins/comfyAPIPlugin.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
build/plugins/comfyAPIPlugin.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Tests should be cross-platform compatible using `path.resolve`, `path.join`, and `path.sep` for Windows, macOS, and Linux compatibility
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Tests should be cross-platform compatible using `path.resolve`, `path.join`, and `path.sep` for Windows, macOS, and Linux compatibility
Applied to files:
build/plugins/comfyAPIPlugin.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/{services,composables}/**/*.{ts,tsx} : Use `api.fileURL()` for static file access instead of constructing URLs directly
Applied to files:
build/plugins/comfyAPIPlugin.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: deploy-and-comment
- GitHub Check: setup
- GitHub Check: test
- GitHub Check: collect
- GitHub Check: lint-and-format
🔇 Additional comments (1)
build/plugins/comfyAPIPlugin.ts (1)
90-97: Path normalization atrelativePathis correct and fixes the Windows escape issueNormalizing
relativePathwith.replace(/\\/g, '/')right afterpath.relativeis the right place to address Windows backslashes: it keepsshimFileName, the// Shim for ...comment, and the interpolated warning message free of\u-like escape sequences while remaining a no‑op on POSIX. This also standardizes the shim asset paths across platforms.Please confirm the plugin behavior by running a build on both Windows and a Unix‑like environment to ensure the emitted shim paths and warnings look as expected.
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.18 MB (baseline 3.18 MB) • ⚪ 0 BMain entry bundles and manifests
Graph Workspace — 956 kB (baseline 956 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Panels & Settings — 298 kB (baseline 298 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
UI Components — 139 kB (baseline 139 kB) • ⚪ 0 BReusable component library chunks
Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Utilities & Hooks — 2.94 kB (baseline 2.94 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 8.56 MB (baseline 8.56 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.84 MB (baseline 3.84 MB) • ⚪ 0 BBundles that do not match a named category
|
…ility (#7087) ## Summary Normalize the path separators by replacing all backslashes with forward slashes before using relativePath Root Cause In build/plugins/comfyAPIPlugin.ts, the path.relative() function generates relative paths that contain backslashes on Windows (e.g., scripts\ui.ts). When this path is directly embedded into a JavaScript string literal, the \u sequence is interpreted as the start of a Unicode escape sequence, causing the SyntaxError: Invalid Unicode escape sequence error ## Screenshots previous error <img width="720" height="208" alt="image" src="https://github.com/user-attachments/assets/45b9a6e1-f35e-473d-af29-9e181bbe24eb" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7087-fix-normalize-path-separators-in-comfyAPIPlugin-for-Windows-compatibility-2bd6d73d365081cb88c1c6a0f168a5b5) by [Unito](https://www.unito.io)
…ility (#7087) ## Summary Normalize the path separators by replacing all backslashes with forward slashes before using relativePath Root Cause In build/plugins/comfyAPIPlugin.ts, the path.relative() function generates relative paths that contain backslashes on Windows (e.g., scripts\ui.ts). When this path is directly embedded into a JavaScript string literal, the \u sequence is interpreted as the start of a Unicode escape sequence, causing the SyntaxError: Invalid Unicode escape sequence error ## Screenshots previous error <img width="720" height="208" alt="image" src="https://github.com/user-attachments/assets/45b9a6e1-f35e-473d-af29-9e181bbe24eb" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7087-fix-normalize-path-separators-in-comfyAPIPlugin-for-Windows-compatibility-2bd6d73d365081cb88c1c6a0f168a5b5) by [Unito](https://www.unito.io)
… for Windows compatibility (#7089) Backport of #7087 to `core/1.33` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7089-backport-core-1-33-fix-normalize-path-separators-in-comfyAPIPlugin-for-Windows-compati-2bd6d73d3650817e83d5d2cc101dc9a1) by [Unito](https://www.unito.io) Co-authored-by: Terry Jia <[email protected]>
…n for Windows compatibility (#7090) Backport of #7087 to `cloud/1.33` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7090-backport-cloud-1-33-fix-normalize-path-separators-in-comfyAPIPlugin-for-Windows-compat-2bd6d73d365081228497fa23b58b8978) by [Unito](https://www.unito.io) Co-authored-by: Terry Jia <[email protected]>
Summary
Normalize the path separators by replacing all backslashes with forward slashes before using relativePath
Root Cause
In build/plugins/comfyAPIPlugin.ts, the path.relative() function generates relative paths that contain backslashes on Windows (e.g., scripts\ui.ts). When this path is directly embedded into a JavaScript string literal, the \u sequence is interpreted as the start of a Unicode escape sequence, causing the SyntaxError: Invalid Unicode escape sequence error
Screenshots
previous error

┆Issue is synchronized with this Notion page by Unito