From 32da91ea32f792585c01c7375bdc10de5689c894 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Tue, 3 Feb 2026 15:15:37 -0800 Subject: [PATCH] perf: code-split xterm bundle and gate terminal features for cloud (#8528) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Code-splits the ~400KB xterm bundle and excludes terminal features from cloud distribution where they are not needed. ### Changes - **bottomPanelStore.ts**: Gate terminal tab registration behind `__DISTRIBUTION__ !== 'cloud'` check with dynamic import, enabling tree-shaking - **keybindingService.ts**: Skip logs-terminal keybinding registration for cloud distribution - **vite.config.mts**: Add `vendor-xterm` code splitting group ### Bundle Impact | Distribution | xterm in bundle | Terminal tabs | |--------------|-----------------|---------------| | cloud | ❌ No (~400KB saved) | None | | localhost | ✅ Yes (vendor-xterm chunk) | Logs terminal | | desktop | ✅ Yes (vendor-xterm chunk) | Logs + Command terminal | ### Verification Script Run locally to verify xterm exclusion works correctly: ```bash #!/bin/bash set -e echo "=== Verifying xterm bundle exclusion ===" echo # Clear Nx cache to ensure fresh builds pnpm nx reset 2>/dev/null # Build cloud distribution echo "Building CLOUD distribution..." rm -rf dist DISTRIBUTION=cloud pnpm build --mode production 2>/dev/null echo "Cloud build - checking for xterm:" if grep -r "xterm" dist/assets/*.js >/dev/null 2>&1; then echo " ❌ FAIL: xterm found in cloud bundle" grep -l "xterm" dist/assets/*.js | head -5 else echo " ✅ PASS: xterm NOT in cloud bundle" fi echo # Build localhost distribution echo "Building LOCALHOST distribution..." pnpm nx reset 2>/dev/null rm -rf dist DISTRIBUTION=localhost pnpm build --mode production 2>/dev/null echo "Localhost build - checking for xterm:" if grep -r "xterm" dist/assets/*.js >/dev/null 2>&1; then echo " ✅ PASS: xterm found in localhost bundle" echo " Files containing xterm:" grep -l "xterm" dist/assets/*.js | while read f; do size=$(wc -c < "$f") echo " $(basename $f) ($(numfmt --to=iec $size))" done else echo " ❌ FAIL: xterm NOT in localhost bundle" fi echo echo "=== Verification complete ===" ``` **Note**: Nx cache must be reset between builds with different `DISTRIBUTION` values or it may return stale results. ## Test Plan - [x] Quality gates pass (typecheck, lint, format, tests) - [x] Cloud build verified: xterm NOT present - [x] Localhost build verified: xterm present as vendor-xterm chunk Fixes COM-14129 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8528-perf-code-split-xterm-bundle-and-gate-terminal-features-for-cloud-2fa6d73d365081a093ecc74ca7ce6e7c) by [Unito](https://www.unito.io) ## Summary by CodeRabbit * **Refactor** * Terminal tab declarations adjusted (no user-visible behavior change). * Bottom panel tabs now load asynchronously; panel toggle falls back to shortcuts when terminal tabs aren’t loaded. * **Chores** * Terminal tabs and related keybindings suppressed in cloud deployments. * Core commands wired to a new utility to support widget promotion. * **Tests** * E2E tests updated to handle async terminal-tab loading and minor test cleanup. --------- Co-authored-by: Amp Co-authored-by: GitHub Action Co-authored-by: Alexander Brown --- .../tests/bottomPanelShortcuts.spec.ts | 36 ++++++++++++++--- browser_tests/tests/menu.spec.ts | 1 - .../bottomPanelTabs/useTerminalTabs.ts | 14 +++---- src/composables/useCoreCommands.ts | 1 + src/platform/keybindings/keybindingService.ts | 13 ++++++ src/stores/workspace/bottomPanelStore.ts | 40 ++++++++++++++----- src/views/GraphView.vue | 2 +- 7 files changed, 81 insertions(+), 26 deletions(-) diff --git a/browser_tests/tests/bottomPanelShortcuts.spec.ts b/browser_tests/tests/bottomPanelShortcuts.spec.ts index d3493f6996..fba61ebd4b 100644 --- a/browser_tests/tests/bottomPanelShortcuts.spec.ts +++ b/browser_tests/tests/bottomPanelShortcuts.spec.ts @@ -113,21 +113,45 @@ test.describe('Bottom Panel Shortcuts', { tag: '@ui' }, () => { expect(hasModifiers).toBeTruthy() }) - test('should maintain panel state when switching to terminal', async ({ + test('should maintain panel state when switching between panels', async ({ comfyPage }) => { const { bottomPanel } = comfyPage + // Open shortcuts panel first await bottomPanel.keyboardShortcutsButton.click() await expect(bottomPanel.root).toBeVisible() - - await bottomPanel.toggleButton.click() - await expect(bottomPanel.root).toBeVisible() - - await bottomPanel.keyboardShortcutsButton.click() await expect( comfyPage.page.locator('[id*="tab_shortcuts-essentials"]') ).toBeVisible() + + // Try to open terminal panel - may show terminal OR close shortcuts + // depending on whether terminal tabs have loaded (async loading) + await bottomPanel.toggleButton.click() + + // Check if terminal tabs loaded (Logs tab visible) or fell back to shortcuts toggle + const logsTab = comfyPage.page.getByRole('tab', { name: /Logs/i }) + const hasTerminalTabs = await logsTab.isVisible().catch(() => false) + + if (hasTerminalTabs) { + // Terminal panel is visible - verify we can switch back to shortcuts + await expect(bottomPanel.root).toBeVisible() + + // Switch back to shortcuts + await bottomPanel.keyboardShortcutsButton.click() + + // Should show shortcuts content again + await expect( + comfyPage.page.locator('[id*="tab_shortcuts-essentials"]') + ).toBeVisible() + } else { + // Terminal tabs not loaded - button toggled shortcuts off, reopen for verification + await bottomPanel.keyboardShortcutsButton.click() + await expect(bottomPanel.root).toBeVisible() + await expect( + comfyPage.page.locator('[id*="tab_shortcuts-essentials"]') + ).toBeVisible() + } }) test('should handle keyboard navigation', async ({ comfyPage }) => { diff --git a/browser_tests/tests/menu.spec.ts b/browser_tests/tests/menu.spec.ts index d028870134..50005afc55 100644 --- a/browser_tests/tests/menu.spec.ts +++ b/browser_tests/tests/menu.spec.ts @@ -107,7 +107,6 @@ test.describe('Menu', { tag: '@ui' }, () => { // Checkmark should be invisible initially (panel is hidden) await expect(checkmark).toHaveClass(/invisible/) - // Click Bottom Panel to toggle it on await bottomPanelItem.click() // Verify menu is still visible after clicking diff --git a/src/composables/bottomPanelTabs/useTerminalTabs.ts b/src/composables/bottomPanelTabs/useTerminalTabs.ts index 2abc79c938..03b16e675c 100644 --- a/src/composables/bottomPanelTabs/useTerminalTabs.ts +++ b/src/composables/bottomPanelTabs/useTerminalTabs.ts @@ -1,27 +1,27 @@ import { markRaw } from 'vue' import { useI18n } from 'vue-i18n' -import CommandTerminal from '@/components/bottomPanel/tabs/terminal/CommandTerminal.vue' import LogsTerminal from '@/components/bottomPanel/tabs/terminal/LogsTerminal.vue' +import CommandTerminal from '@/components/bottomPanel/tabs/terminal/CommandTerminal.vue' import type { BottomPanelExtension } from '@/types/extensionTypes' -export const useLogsTerminalTab = (): BottomPanelExtension => { +export function useLogsTerminalTab(): BottomPanelExtension { const { t } = useI18n() return { id: 'logs-terminal', - title: t('g.logs'), // For command labels (collected by i18n workflow) - titleKey: 'g.logs', // For dynamic translation in UI + title: t('g.logs'), + titleKey: 'g.logs', component: markRaw(LogsTerminal), type: 'vue' } } -export const useCommandTerminalTab = (): BottomPanelExtension => { +export function useCommandTerminalTab(): BottomPanelExtension { const { t } = useI18n() return { id: 'command-terminal', - title: t('g.terminal'), // For command labels (collected by i18n workflow) - titleKey: 'g.terminal', // For dynamic translation in UI + title: t('g.terminal'), + titleKey: 'g.terminal', component: markRaw(CommandTerminal), type: 'vue' } diff --git a/src/composables/useCoreCommands.ts b/src/composables/useCoreCommands.ts index 6a8153a28d..644d33ac95 100644 --- a/src/composables/useCoreCommands.ts +++ b/src/composables/useCoreCommands.ts @@ -8,6 +8,7 @@ import { DEFAULT_DARK_COLOR_PALETTE, DEFAULT_LIGHT_COLOR_PALETTE } from '@/constants/coreColorPalettes' + import { tryToggleWidgetPromotion } from '@/core/graph/subgraph/proxyWidgetUtils' import { t } from '@/i18n' import { diff --git a/src/platform/keybindings/keybindingService.ts b/src/platform/keybindings/keybindingService.ts index 31552ed437..f8c822273e 100644 --- a/src/platform/keybindings/keybindingService.ts +++ b/src/platform/keybindings/keybindingService.ts @@ -1,3 +1,4 @@ +import { isCloud } from '@/platform/distribution/types' import { useSettingStore } from '@/platform/settings/settingStore' import { app } from '@/scripts/app' import { useCommandStore } from '@/stores/commandStore' @@ -108,6 +109,12 @@ export function useKeybindingService() { function registerCoreKeybindings() { for (const keybinding of CORE_KEYBINDINGS) { + if ( + isCloud && + keybinding.commandId === 'Workspace.ToggleBottomPanelTab.logs-terminal' + ) { + continue + } keybindingStore.addDefaultKeybinding(new KeybindingImpl(keybinding)) } } @@ -119,6 +126,12 @@ export function useKeybindingService() { } const newBindings = settingStore.get('Comfy.Keybinding.NewBindings') for (const keybinding of newBindings) { + if ( + isCloud && + keybinding.commandId === 'Workspace.ToggleBottomPanelTab.logs-terminal' + ) { + continue + } keybindingStore.addUserKeybinding(new KeybindingImpl(keybinding)) } } diff --git a/src/stores/workspace/bottomPanelStore.ts b/src/stores/workspace/bottomPanelStore.ts index 017c2e0b29..5a5e7090ea 100644 --- a/src/stores/workspace/bottomPanelStore.ts +++ b/src/stores/workspace/bottomPanelStore.ts @@ -2,10 +2,7 @@ import { defineStore } from 'pinia' import { computed, ref } from 'vue' import { useShortcutsTab } from '@/composables/bottomPanelTabs/useShortcutsTab' -import { - useCommandTerminalTab, - useLogsTerminalTab -} from '@/composables/bottomPanelTabs/useTerminalTabs' + import { useCommandStore } from '@/stores/commandStore' import type { ComfyExtension } from '@/types/comfy' import type { BottomPanelExtension } from '@/types/extensionTypes' @@ -75,8 +72,18 @@ export const useBottomPanelStore = defineStore('bottomPanel', () => { } const toggleBottomPanel = () => { - // Legacy method - toggles terminal panel - togglePanel('terminal') + // Toggles the terminal panel if available, otherwise falls back to shortcuts + // Terminal tabs are loaded asynchronously, so may not be available immediately + const terminalPanel = panels.value.terminal + if (terminalPanel.tabs.length > 0) { + togglePanel('terminal') + } else { + // Terminal tabs not loaded yet - fall back to shortcuts panel + // If no panel is open, open shortcuts + // If shortcuts is already open, close it + // If another panel is open (shouldn't happen), switch to shortcuts + togglePanel('shortcuts') + } } const setActiveTab = (tabId: string) => { @@ -121,12 +128,23 @@ export const useBottomPanelStore = defineStore('bottomPanel', () => { }) } - const registerCoreBottomPanelTabs = () => { - registerBottomPanelTab(useLogsTerminalTab()) - if (isElectron()) { - registerBottomPanelTab(useCommandTerminalTab()) - } + const registerCoreBottomPanelTabs = async () => { + // Register shortcuts tabs first (synchronous, always available) useShortcutsTab().forEach(registerBottomPanelTab) + + // Use __DISTRIBUTION__ directly for proper dead code elimination + if (__DISTRIBUTION__ !== 'cloud') { + try { + const { useLogsTerminalTab, useCommandTerminalTab } = + await import('@/composables/bottomPanelTabs/useTerminalTabs') + registerBottomPanelTab(useLogsTerminalTab()) + if (isElectron()) { + registerBottomPanelTab(useCommandTerminalTab()) + } + } catch (error) { + console.error('Failed to load terminal tabs:', error) + } + } } const registerExtensionBottomPanelTabs = (extension: ComfyExtension) => { diff --git a/src/views/GraphView.vue b/src/views/GraphView.vue index bbe22d1a1a..4b5ec6d4a5 100644 --- a/src/views/GraphView.vue +++ b/src/views/GraphView.vue @@ -198,7 +198,7 @@ useCommandStore().registerCommands(coreCommands) useMenuItemStore().registerCoreMenuCommands() useKeybindingService().registerCoreKeybindings() useSidebarTabStore().registerCoreSidebarTabs() -useBottomPanelStore().registerCoreBottomPanelTabs() +void useBottomPanelStore().registerCoreBottomPanelTabs() const queuePendingTaskCountStore = useQueuePendingTaskCountStore() const sidebarTabStore = useSidebarTabStore()