-
Notifications
You must be signed in to change notification settings - Fork 490
refactor: parallelize bootstrap and simplify lifecycle with VueUse #8307
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
Changes from all commits
8afb45d
33c457a
326154f
0a73c5a
3c0b227
80d58dd
52483b3
51bce92
c171227
35eb028
16b285a
0ca910a
07e6daa
b11eb7e
69efc65
d09f948
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,7 +93,7 @@ | |
| </template> | ||
|
|
||
| <script setup lang="ts"> | ||
| import { useEventListener, whenever } from '@vueuse/core' | ||
| import { until, useEventListener } from '@vueuse/core' | ||
| import { | ||
| computed, | ||
| nextTick, | ||
|
|
@@ -129,7 +129,7 @@ import { useCopy } from '@/composables/useCopy' | |
| import { useGlobalLitegraph } from '@/composables/useGlobalLitegraph' | ||
| import { usePaste } from '@/composables/usePaste' | ||
| import { useVueFeatureFlags } from '@/composables/useVueFeatureFlags' | ||
| import { mergeCustomNodesI18n, t } from '@/i18n' | ||
| import { t } from '@/i18n' | ||
| import { LiteGraph } from '@/lib/litegraph/src/litegraph' | ||
| import { useLitegraphSettings } from '@/platform/settings/composables/useLitegraphSettings' | ||
| import { CORE_SETTINGS } from '@/platform/settings/constants/coreSettings' | ||
|
|
@@ -144,12 +144,15 @@ import { useCanvasInteractions } from '@/renderer/core/canvas/useCanvasInteracti | |
| import TransformPane from '@/renderer/core/layout/transform/TransformPane.vue' | ||
| import MiniMap from '@/renderer/extensions/minimap/MiniMap.vue' | ||
| import LGraphNode from '@/renderer/extensions/vueNodes/components/LGraphNode.vue' | ||
| import { UnauthorizedError, api } from '@/scripts/api' | ||
| import { UnauthorizedError } from '@/scripts/api' | ||
| import { app as comfyApp } from '@/scripts/app' | ||
| import { ChangeTracker } from '@/scripts/changeTracker' | ||
| import { IS_CONTROL_WIDGET, updateControlWidgetLabel } from '@/scripts/widgets' | ||
| import { useColorPaletteService } from '@/services/colorPaletteService' | ||
| import { newUserService } from '@/services/newUserService' | ||
| import { storeToRefs } from 'pinia' | ||
|
|
||
| import { useBootstrapStore } from '@/stores/bootstrapStore' | ||
| import { useCommandStore } from '@/stores/commandStore' | ||
| import { useExecutionStore } from '@/stores/executionStore' | ||
| import { useNodeDefStore } from '@/stores/nodeDefStore' | ||
|
|
@@ -175,11 +178,16 @@ const settingStore = useSettingStore() | |
| const nodeDefStore = useNodeDefStore() | ||
| const workspaceStore = useWorkspaceStore() | ||
| const canvasStore = useCanvasStore() | ||
| const workflowStore = useWorkflowStore() | ||
| const executionStore = useExecutionStore() | ||
| const toastStore = useToastStore() | ||
| const colorPaletteStore = useColorPaletteStore() | ||
| const colorPaletteService = useColorPaletteService() | ||
| const canvasInteractions = useCanvasInteractions() | ||
| const bootstrapStore = useBootstrapStore() | ||
| const { isI18nReady, i18nError } = storeToRefs(bootstrapStore) | ||
| const { isReady: isSettingsReady, error: settingsError } = | ||
| storeToRefs(settingStore) | ||
|
|
||
| const betaMenuEnabled = computed( | ||
| () => settingStore.get('Comfy.UseNewMenu') !== 'Disabled' | ||
|
|
@@ -386,15 +394,6 @@ useEventListener( | |
| { passive: true } | ||
| ) | ||
|
|
||
| const loadCustomNodesI18n = async () => { | ||
| try { | ||
| const i18nData = await api.getCustomNodesI18n() | ||
| mergeCustomNodesI18n(i18nData) | ||
| } catch (error) { | ||
| console.error('Failed to load custom nodes i18n', error) | ||
| } | ||
| } | ||
|
|
||
| const comfyAppReady = ref(false) | ||
| const workflowPersistence = useWorkflowPersistence() | ||
| const { flags } = useFeatureFlags() | ||
|
|
@@ -404,35 +403,72 @@ useCanvasDrop(canvasRef) | |
| useLitegraphSettings() | ||
| useNodeBadge() | ||
|
|
||
| onMounted(async () => { | ||
| useGlobalLitegraph() | ||
| useContextMenuTranslation() | ||
| useCopy() | ||
| usePaste() | ||
| useWorkflowAutoSave() | ||
| useVueFeatureFlags() | ||
| useGlobalLitegraph() | ||
| useContextMenuTranslation() | ||
| useCopy() | ||
| usePaste() | ||
| useWorkflowAutoSave() | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| comfyApp.vueAppReady = true | ||
| // Start watching for locale change after the initial value is loaded. | ||
| watch( | ||
| () => settingStore.get('Comfy.Locale'), | ||
| async (_newLocale, oldLocale) => { | ||
| if (!oldLocale) return | ||
| await Promise.all([ | ||
| until(() => isSettingsReady.value || !!settingsError.value).toBe(true), | ||
| until(() => isI18nReady.value || !!i18nError.value).toBe(true) | ||
| ]) | ||
| if (settingsError.value || i18nError.value) { | ||
| console.warn( | ||
| 'Somehow the Locale setting was changed while the settings or i18n had a setup error' | ||
| ) | ||
| } | ||
| await useCommandStore().execute('Comfy.RefreshNodeDefinitions') | ||
| await useWorkflowService().reloadCurrentWorkflow() | ||
| } | ||
| ) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worth adding an There's some race condition potential here, but I think the chances of it occurring are near nonexistant.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Possibly, but I'm not sure what I'd be cleaning up yet.
DrJKL marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| useEventListener( | ||
| () => canvasStore.canvas?.canvas, | ||
| 'litegraph:set-graph', | ||
| () => { | ||
| workflowStore.updateActiveGraph() | ||
| } | ||
| ) | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| onMounted(async () => { | ||
| comfyApp.vueAppReady = true | ||
| workspaceStore.spinner = true | ||
| // ChangeTracker needs to be initialized before setup, as it will overwrite | ||
| // some listeners of litegraph canvas. | ||
| ChangeTracker.init() | ||
| await loadCustomNodesI18n() | ||
| try { | ||
| await settingStore.loadSettingValues() | ||
| } catch (error) { | ||
| if (error instanceof UnauthorizedError) { | ||
|
|
||
| await until(() => isSettingsReady.value || !!settingsError.value).toBe(true) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this logic/nuance be abstracted and exported from the store? So it's used like
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. I considered doing that for each of the lazily initialized stores instead of keeping the logic in bootstrap. I'll try it out.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just lacking an analogous store for i18n pieces. |
||
|
|
||
| if (settingsError.value) { | ||
| if (settingsError.value instanceof UnauthorizedError) { | ||
| localStorage.removeItem('Comfy.userId') | ||
| localStorage.removeItem('Comfy.userName') | ||
| window.location.reload() | ||
| } else { | ||
| throw error | ||
| return | ||
| } | ||
| throw settingsError.value | ||
| } | ||
|
|
||
| // Register core settings immediately after settings are ready | ||
| CORE_SETTINGS.forEach(settingStore.addSetting) | ||
|
|
||
| await newUserService().initializeIfNewUser(settingStore) | ||
| // Wait for both i18n and newUserService in parallel | ||
| // (newUserService only needs settings, not i18n) | ||
| await Promise.all([ | ||
| until(() => isI18nReady.value || !!i18nError.value).toBe(true), | ||
| newUserService().initializeIfNewUser(settingStore) | ||
| ]) | ||
| if (i18nError.value) { | ||
| console.warn( | ||
| '[GraphCanvas] Failed to load custom nodes i18n:', | ||
| i18nError.value | ||
| ) | ||
| } | ||
christian-byrne marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // @ts-expect-error fixme ts strict error | ||
| await comfyApp.setup(canvasRef.value) | ||
|
|
@@ -487,25 +523,6 @@ onMounted(async () => { | |
| const releaseStore = useReleaseStore() | ||
| void releaseStore.initialize() | ||
|
|
||
| // Start watching for locale change after the initial value is loaded. | ||
| watch( | ||
| () => settingStore.get('Comfy.Locale'), | ||
| async () => { | ||
| await useCommandStore().execute('Comfy.RefreshNodeDefinitions') | ||
| await useWorkflowService().reloadCurrentWorkflow() | ||
| } | ||
| ) | ||
|
|
||
| whenever( | ||
| () => useCanvasStore().canvas, | ||
| (canvas) => { | ||
| useEventListener(canvas.canvas, 'litegraph:set-graph', () => { | ||
| useWorkflowStore().updateActiveGraph() | ||
| }) | ||
| }, | ||
| { immediate: true } | ||
| ) | ||
|
|
||
| emit('ready') | ||
| }) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,6 +71,9 @@ export const useNodeBadge = () => { | |
| } | ||
|
|
||
| onMounted(() => { | ||
| if (extensionStore.isExtensionInstalled('Comfy.NodeBadge')) return | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
|
||
| // TODO: Fix the composables and watchers being setup in onMounted | ||
| const nodePricing = useNodePricing() | ||
|
|
||
| watch( | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.