refactor: parallelize bootstrap and simplify lifecycle with VueUse#8307
Merged
refactor: parallelize bootstrap and simplify lifecycle with VueUse#8307
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a new Pinia bootstrap store to orchestrate startup (i18n, user init, conditional settings/workflow loading, optional Firebase gating); converts several stores to async readiness APIs, changes keyboard tracking to useMagicKeys, exposes workspace store at runtime, refactors graph/view init to VueUse patterns, updates settings/workflow loading semantics, tests, and docs. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App (main.ts / App.vue)
participant Pinia as Pinia
participant Bootstrap as BootstrapStore
participant User as UserStore
participant I18n as i18n Module
participant Settings as Settings Store
participant Workflow as Workflow Store
participant Firebase as Firebase Auth (optional)
App->>Pinia: createPinia()
App->>Bootstrap: useBootstrapStore(pinia)
App->>Bootstrap: startStoreBootstrap()
activate Bootstrap
Bootstrap->>User: initialize user store
User-->>Bootstrap: auth state (needsLogin)
par load i18n
Bootstrap->>I18n: load & merge custom nodes i18n
I18n-->>Bootstrap: ready / error
and conditional settings & workflows
alt user authenticated or no-login required
Bootstrap->>Settings: Settings.load()
Settings-->>Bootstrap: isReady
Bootstrap->>Workflow: loadWorkflows()
Workflow-->>Bootstrap: ready
else defer until auth
end
end
opt cloud distribution
Bootstrap->>Firebase: wait for initialization
Firebase-->>Bootstrap: initialized
end
Bootstrap-->>App: bootstrap complete
deactivate Bootstrap
App->>Workflow: rely on isReady / loadWorkflows before rendering graphs
App->>I18n: rely on isI18nReady before node definitions
Possibly related PRs
Suggested reviewers
✨ Finishing touches
Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 01/27/2026, 08:25:50 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Tests: ✅ PassedResults: 507 passed, 0 failed, 0 flaky, 8 skipped (Total: 515) 📊 Browser Reports
|
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 22.9 kB (baseline 22.8 kB) • 🔴 +175 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 960 kB (baseline 960 kB) • 🟢 -384 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 80.7 kB (baseline 80.7 kB) • 🔴 +1 BTop-level views, pages, and routed surfaces
Status: 11 added / 11 removed Panels & Settings — 470 kB (baseline 470 kB) • 🔴 +16 BConfiguration panels, inspectors, and settings screens
Status: 24 added / 24 removed User & Accounts — 3.94 kB (baseline 3.94 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 3 added / 3 removed Editors & Dialogs — 2.86 kB (baseline 2.83 kB) • 🔴 +24 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 33.7 kB (baseline 33.7 kB) • 🔴 +2 BReusable component library chunks
Status: 9 added / 9 removed Data & Services — 2.7 MB (baseline 3.19 MB) • 🟢 -492 kBStores, services, APIs, and repositories
Status: 10 added / 9 removed Utilities & Hooks — 25.2 kB (baseline 25.2 kB) • 🔴 +74 BHelpers, composables, and utility bundles
Status: 11 added / 11 removed Vendor & Third-Party — 10.7 MB (baseline 10.7 MB) • 🔴 +4.19 kBExternal libraries and shared vendor chunks
Status: 8 added / 8 removed Other — 7.04 MB (baseline 6.55 MB) • 🔴 +496 kBBundles that do not match a named category
Status: 91 added / 89 removed |
DrJKL
force-pushed
the
drjkl/first-things-first
branch
2 times, most recently
from
549f3f5 to
14a6a9a
Compare
- Add useBootstrapStore to centralize early initialization (api.init, fetchNodeDefs) - Move settings loading and custom nodes i18n loading to store bootstrap phase - Use VueUse's `until` to coordinate async dependencies in GraphCanvas - Load settings, i18n, and newUserService initialization in parallel where possible - Add unit tests for bootstrapStore Amp-Thread-ID: https://ampcode.com/threads/T-019bf48d-af90-738f-99ce-46309e4be688 Co-authored-by: Amp <amp@ampcode.com>
DrJKL
force-pushed
the
drjkl/first-things-first
branch
from
14a6a9a to
326154f
Compare
Amp-Thread-ID: https://ampcode.com/threads/T-019bf491-17c4-75d1-a218-85842c463232 Co-authored-by: Amp <amp@ampcode.com>
Contributor
christian-byrne
left a comment
christian-byrne
left a comment
There was a problem hiding this comment.
No multi layered DI containers and bootstrap.ts per distribution? 😭
- Replace manual keydown/keyup listeners with useMagicKeys for shiftDown - Move app.extensionManager assignment to App.vue - Remove init() wrapper function in GraphView.vue Amp-Thread-ID: https://ampcode.com/threads/T-019bf90e-1cd0-71de-b23f-ea6cb0f99a87 Co-authored-by: Amp <amp@ampcode.com>
DrJKL
changed the title
DrJKL
force-pushed
the
drjkl/first-things-first
branch
from
39a7ecb to
3c0b227
Compare
- Move useCopy, usePaste, useWorkflowAutoSave to top level - Move locale change watcher outside onMounted with proper guard - Move litegraph:set-graph event listener to top level Amp-Thread-ID: https://ampcode.com/threads/T-019bf94d-b905-77ee-965d-a9c2a327dc2a Co-authored-by: Amp <amp@ampcode.com>
DrJKL
force-pushed
the
drjkl/first-things-first
branch
from
8cfa4bd to
80d58dd
Compare
dosubot
bot
added
the
size:L
coderabbitai
bot
requested review from
KarryCharon,
Yorha4D,
benceruleanlu and
shinshin86
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/composables/node/useNodeBadge.ts (1)
73-85: Guard skips pricing watcher on remount.If the composable remounts while the extension is already installed, the early return prevents the pricing watch from being re-registered, which can leave badges stale. Consider moving the guard after the pricing watch setup.
🔧 Suggested adjustment
- onMounted(() => { - if (extensionStore.isExtensionInstalled('Comfy.NodeBadge')) return - - // TODO: Fix the composables and watchers being setup in onMounted - const nodePricing = useNodePricing() - - watch( - () => nodePricing.pricingRevision.value, - () => { - if (!showApiPricingBadge.value) return - app.canvas?.setDirty(true, true) - } - ) + onMounted(() => { + // TODO: Fix the composables and watchers being setup in onMounted + const nodePricing = useNodePricing() + + watch( + () => nodePricing.pricingRevision.value, + () => { + if (!showApiPricingBadge.value) return + app.canvas?.setDirty(true, true) + } + ) + + if (extensionStore.isExtensionInstalled('Comfy.NodeBadge')) returnsrc/views/GraphView.vue (1)
304-360: Close BroadcastChannel and dispose of runWhenGlobalIdle on unmount.The
BroadcastChannelcreated insideonGraphReadyis never closed, causing a resource leak. Additionally, therunWhenGlobalIdlereturn value is never captured or disposed. If the component remounts (via route changes), stale idle tasks could execute. Capture the idle task disposable and channel reference, then clean them up inonBeforeUnmount.🧹 One way to handle explicit cleanup
const onGraphReady = () => { - runWhenGlobalIdle(() => { + const idleDisposable = runWhenGlobalIdle(() => { if (isCloud && telemetry) { const tabCountChannel = new BroadcastChannel('comfyui-tab-count') const activeTabs = new Map<string, number>() const currentTabId = crypto.randomUUID() // ... existing code ... tabCountChannel.postMessage({ type: 'heartbeat', tabId: currentTabId }) + + onBeforeUnmount(() => { + tabCountChannel.close() + }) } - }, 1000) + }, 1000) + + return { idleDisposable } } onBeforeUnmount(() => { executionStore.unbindExecutionEvents() + const { idleDisposable } = onGraphReady() + idleDisposable?.dispose() })Better approach: store the idle disposable and channel reference as component-level variables and dispose them in
onBeforeUnmount.
🤖 Fix all issues with AI agents
In `@src/components/graph/GraphCanvas.vue`:
- Around line 404-409: The second invocation of the composable
useVueFeatureFlags in GraphCanvas.vue is redundant (it was already called
earlier to compute shouldRenderVueNodes) and can create duplicate watchers;
remove the extra call from the bottom composable calls (the listed
useVueFeatureFlags() alongside
useGlobalLitegraph/useContextMenuTranslation/etc.) and ensure any
feature-flag-dependent logic continues to reference the single earlier return
value used for shouldRenderVueNodes so no other code relies on the removed
invocation.
- Around line 411-424: The watch handler for settingStore.get('Comfy.Locale')
must short-circuit when settings failed: check settingsError (and/or
isSettingsReady) after awaiting the initial until and if settingsError is truthy
return early (or call the existing Unauthorized handling) before proceeding to
await isI18nReady/i18nError, calling newUserService().initializeIfNewUser,
useCommandStore().execute('Comfy.RefreshNodeDefinitions'), or
useWorkflowService().reloadCurrentWorkflow; update the watcher logic around
isSettingsReady and settingsError to abort when settingsError is set to avoid
operating on invalid state.
In `@src/platform/settings/settingStore.ts`:
- Around line 172-177: When a duplicate registration is detected at the `if
(setting.id in settingsById.value)` check, update the stored SettingParams
instead of returning immediately so remounts that pass new callbacks/options
don't keep stale references; locate the block that references
`settingsById.value[setting.id]` and assign or merge the incoming `setting` (or
at minimum its callbacks/options) into the existing entry before returning
(e.g., replace or shallow-merge into `settingsById.value[setting.id]`) so the
store reflects the latest definition.
In `@src/stores/bootstrapStore.test.ts`:
- Around line 6-31: The test is missing a mock for the user store dependency,
causing startStoreBootstrap() (which awaits useUserStore().initialize() and
branches on needsLogin) to be flaky; add a vi.mock for the module that exports
useUserStore so the mocked useUserStore returns an object with initialize:
vi.fn().mockResolvedValue(undefined) and needsLogin explicitly set (e.g., false)
so startStoreBootstrap() behaves deterministically when invoking
useUserStore().initialize() and checking needsLogin.
In `@src/stores/bootstrapStore.ts`:
- Around line 61-78: The async workflow sync currently ignores errors;
destructure the error ref from the useAsyncState call (e.g., const { isReady:
isWorkflowsReady, isLoading: isWorkflowsLoading, execute: executeSyncWorkflows,
error: syncWorkflowsError } = useAsyncState(...)) and add syncWorkflowsError to
the bootstrap store's returned object so callers can observe failures from
syncWorkflows; keep the existing
isWorkflowsReady/isWorkflowsLoading/executeSyncWorkflows and syncWorkflows logic
unchanged.
c491857 to
35eb028
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/platform/workflow/management/stores/workflowStore.ts`:
- Line 872: The exported WorkflowStore interface is missing the newly returned
loadWorkflows function; update the WorkflowStore interface to include a
loadWorkflows signature alongside syncWorkflows so the exposed store type
matches the actual returned store. Locate the WorkflowStore interface
declaration and add a loadWorkflows: (...) => ... signature consistent with the
existing syncWorkflows method (use the same parameter/return conventions as
syncWorkflows) and update any related type exports if necessary.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/main.ts`:
- Around line 93-96: The code currently fire-and-forgets
bootstrapStore.startStoreBootstrap() which can swallow startup errors; change it
to await or explicitly handle rejections so the app only mounts after successful
bootstrap. Replace the void call with awaiting the promise (e.g., call
startStoreBootstrap() inside an async init function and await it before
app.mount('#vue-app')) or attach a .catch that logs the error and prevents
mounting (using useBootstrapStore and its startStoreBootstrap() result), and
ensure any lower-level failures like userStore.initialize() propagate so you can
abort mount on failure.
In `@src/stores/bootstrapStore.test.ts`:
- Around line 62-76: Add a test that mocks useUserStore to return needsLogin:
true so store.startStoreBootstrap() exercises the login-required branch; import
useUserStore, stub its initialize to resolve and set needsLogin: true via
vi.mocked(useUserStore).mockReturnValueOnce(...), then call
store.startStoreBootstrap() and assert that useSettingStore().isReady remains
false and that workflowStore.loadWorkflows was not invoked, while
store.isI18nReady still becomes true (use vi.waitFor to wait for i18n
readiness).
In `@src/stores/bootstrapStore.ts`:
- Around line 36-39: The sequential awaits for settingStore.load() and
workflowStore.loadWorkflows() inside the userStore.needsLogin check should be
parallelized: instead of awaiting them one after the other, start both calls and
await them together (e.g., with Promise.all) so settingStore.load and
workflowStore.loadWorkflows run concurrently; locate the block guarded by
userStore.needsLogin and replace the sequential awaits with a combined await to
run both promises in parallel while preserving existing error propagation.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
browser_tests/tests/versionMismatchWarnings.spec.ts (1)
106-109: Consider using a more robust locator.The
.nth(3)index-based selector is fragile and could break silently if the DOM structure changes. Consider using a more specific selector like adata-testidattribute or a more targeted locator chain.♻️ Suggested improvement
- const warningToast = comfyPage.page - .locator('div') - .filter({ hasText: 'Version Compatibility' }) - .nth(3) + const warningToast = comfyPage.page + .locator('[data-testid="version-mismatch-toast"]') + // Or use a more specific semantic locator: + // .getByRole('alert').filter({ hasText: 'Version Compatibility' })This would require adding a
data-testidto the toast component, but would make the test more resilient to UI refactoring.
Amp-Thread-ID: https://ampcode.com/threads/T-019bfd62-e2e1-7797-849b-919a0e8cf4e7 Co-authored-by: Amp <amp@ampcode.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/platform/settings/settingStore.ts`:
- Around line 263-266: Remove the exported async-state symbols from the public
API: stop exporting isReady, isLoading, and error from the setting store and
keep only load() as the public method; keep the variables (isReady, isLoading,
error) as internal/local bindings returned/used inside the store implementation
(e.g., within the same closure that defines useAsyncState in settingStore.ts) so
consumers only see and use load(), and update any internal references to those
symbols to use the now-internal bindings rather than exported names.
- Around line 74-78: The load() function currently returns undefined when
isLoading.value is true, breaking awaitability; change load() to cache the
in-flight Promise returned by loadSettingValues() (e.g. store it in a local
variable like pendingLoadPromise) and return that same Promise for any
concurrent calls while isLoading is true, and ensure the cached promise is
cleared (set to null) when the promise resolves or rejects; reference load(),
loadSettingValues(), isReady and isLoading to locate where to store, return, and
clear the pending promise so multiple callers properly await the same
initialization.
In `@src/stores/bootstrapStore.ts`:
- Around line 29-47: startStoreBootstrap only loads settingStore.load() and
workflowStore.loadWorkflows() when userStore.needsLogin is false at bootstrap,
so stores never initialize when the user logs in later; add a post-auth hook
that observes the needsLogin flag from useUserStore (or call from the login
flow) and triggers settingStore.load() and workflowStore.loadWorkflows() when
needsLogin transitions to false. Specifically, add a watcher / effect that
watches useUserStore().needsLogin (or use storeToRefs on the user store) and, on
change to false, calls settingStore.load() and workflowStore.loadWorkflows();
ensure this logic coexists with the existing bootstrap conditional in
startStoreBootstrap so loads occur either at bootstrap or after login.
Comment on lines
+29
to
+47
| async function startStoreBootstrap() { | ||
| // Defer settings and workflows if multi-user login is required | ||
| // (settings API requires authentication in multi-user mode) | ||
| const userStore = useUserStore() | ||
| await userStore.initialize() | ||
|
|
||
| if (isCloud) { | ||
| const { isInitialized } = storeToRefs(useFirebaseAuthStore()) | ||
| await until(isInitialized).toBe(true) | ||
| } | ||
|
|
||
| // i18n can load without authentication | ||
| void loadI18n() | ||
|
|
||
| if (!userStore.needsLogin) { | ||
| void settingStore.load() | ||
| void workflowStore.loadWorkflows() | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find all usages of settingStore.load()
rg -n --type=ts --type=vue -C3 'settingStore\.load\(' srcRepository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
# Find all usages of workflowStore.loadWorkflows()
rg -n --type=ts --type=vue -C3 'workflowStore\.loadWorkflows\(' srcRepository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
# Look for post-login/auth initialization in user store and related flows
rg -n --type=ts -C5 'initialize\|logout\|needsLogin' src/storesRepository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Search for any watchers or effects on needsLogin in bootstrap or related files
rg -n --type=ts -C3 'needsLogin' src/stores/bootstrapStore.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 266
🏁 Script executed:
# Check if there are any login flow handlers in userStore
fd -e ts src/stores | xargs rg -l 'login' | head -5Repository: Comfy-Org/ComfyUI_frontend
Length of output: 555
🏁 Script executed:
# Find all usages of settingStore.load() - corrected syntax
rg -n 'settingStore\.load\(' src --type tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 132
🏁 Script executed:
# Find all usages of workflowStore.loadWorkflows()
rg -n 'workflowStore\.loadWorkflows\(' src --type tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 142
🏁 Script executed:
# Check userStore for needsLogin and related logic
fd . src/stores -name "*userStore*" -o -name "*user*"Repository: Comfy-Org/ComfyUI_frontend
Length of output: 300
🏁 Script executed:
# Look at bootstrap file more carefully
cat -n src/stores/bootstrapStore.ts | head -60Repository: Comfy-Org/ComfyUI_frontend
Length of output: 2025
🏁 Script executed:
# Search for any watchers or effects on needsLogin in the stores
rg -n 'needsLogin' src/stores --type ts -B2 -A2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1185
🏁 Script executed:
# Check if login completion triggers anything
rg -n 'watch.*needsLogin\|watchEffect.*needsLogin' src --type tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Read the userStore to understand the complete implementation
cat -n src/stores/userStore.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 3340
🏁 Script executed:
# Look for where currentUserId is set/updated
rg -n 'currentUserId' src/stores/userStore.ts -B2 -A2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1153
🏁 Script executed:
# Search for any side effects or watchers on currentUserId change
rg -n 'watch.*currentUserId\|effect.*currentUserId' src --type tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Check router.ts to see if login flow triggers any loads
rg -n 'settingStore\|workflowStore\|load' src/router.ts -B2 -A2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Check app.ts for any post-login initialization
rg -n 'settingStore\|workflowStore\|needsLogin\|login' src/scripts/app.ts -B2 -A2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Search for UserSelectView handling
rg -n 'settingStore\|workflowStore' src/views/UserSelectView.vue -B2 -A2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Check if UserSelectView or login-related views manually trigger loads after login
fd . src/views -type f | xargs rg -l 'login\|settingStore\|workflowStore' --type tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 242
🏁 Script executed:
# Look at UserSelectView in detail to see what happens after login
cat src/views/UserSelectView.vue | head -100Repository: Comfy-Org/ComfyUI_frontend
Length of output: 2930
🏁 Script executed:
# Search the entire codebase for any effect/watch that responds to login completion
rg -n 'login.*then\|\.login\(' src --type ts -A5 | head -50Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Find where startStoreBootstrap is called
rg -n 'startStoreBootstrap' src --type ts -B2 -A2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1186
🏁 Script executed:
# Check main.ts or app initialization
fd . src -name "main.ts" -o -name "App.vue" | head -5Repository: Comfy-Org/ComfyUI_frontend
Length of output: 300
🏁 Script executed:
# Look at main.ts to see bootstrap flow
cat src/main.ts | head -80Repository: Comfy-Org/ComfyUI_frontend
Length of output: 2465
🏁 Script executed:
# Check if there's any router navigation guard or effect that re-triggers loads
rg -n 'beforeEach\|afterEach\|router\.' src/main.ts -A3 -B1Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Look for any component-level effects that check needsLogin
rg -n 'watch.*needsLogin' src --type tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 52
Implement post-auth hook to load deferred settings and workflows.
Settings and workflows are only loaded during initial bootstrap if login is not required. When a user logs in after bootstrap (making needsLogin false), there is no mechanism to trigger settingStore.load() and workflowStore.loadWorkflows(). Add a watcher or effect on needsLogin change, or trigger these loads in the login flow, to ensure stores are initialized after authentication completes.
🤖 Prompt for AI Agents
In `@src/stores/bootstrapStore.ts` around lines 29 - 47, startStoreBootstrap only
loads settingStore.load() and workflowStore.loadWorkflows() when
userStore.needsLogin is false at bootstrap, so stores never initialize when the
user logs in later; add a post-auth hook that observes the needsLogin flag from
useUserStore (or call from the login flow) and triggers settingStore.load() and
workflowStore.loadWorkflows() when needsLogin transitions to false.
Specifically, add a watcher / effect that watches useUserStore().needsLogin (or
use storeToRefs on the user store) and, on change to false, calls
settingStore.load() and workflowStore.loadWorkflows(); ensure this logic
coexists with the existing bootstrap conditional in startStoreBootstrap so loads
occur either at bootstrap or after login.
- Return early if already loaded (isReady/isSyncReady) - Use VueUse `until()` to await in-progress loading instead of returning undefined - Ensures all callers properly wait for initialization to complete Amp-Thread-ID: https://ampcode.com/threads/T-019bfdee-7166-76c3-8c5f-473717a9148b Co-authored-by: Amp <amp@ampcode.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/platform/settings/settingStore.ts`:
- Around line 74-83: The current load() can silently succeed for concurrent
callers because they await until(isLoading).toBe(false) then return even if
loadSettingValues() failed; change load() to track and await the actual
in-flight promise instead of just waiting for isLoading to flip: introduce (or
reuse) a loadPromise variable set when starting loadSettingValues() (e.g., let
loadPromise: Promise<void>), assign loadPromise = loadSettingValues() inside
load(), set isLoading appropriately, await loadPromise for concurrent callers
(await loadPromise) and clear it when done, so callers receive any thrown errors
and isReady is set only on success. Ensure references to isLoading, isReady,
loadSettingValues(), and loadPromise are used consistently.
In `@src/platform/workflow/management/stores/workflowStore.ts`:
- Around line 561-570: loadWorkflows can return silently if a concurrent sync
fails: after awaiting until(isSyncLoading).toBe(false) check whether the sync
actually succeeded (inspect isSyncReady.value) and if not either re-attempt the
sync by calling syncWorkflows() or throw/reject to surface the failure; update
the function (referencing loadWorkflows, isSyncLoading, isSyncReady, until and
syncWorkflows) so callers don’t proceed under a false assumption that workflows
were loaded.
AustinMroz
previously approved these changes
Jan 27, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/extensions/minimap/composables/useMinimapGraph.test.ts (1)
105-109: Consider removing commented-out code.The
originalCallbacksblock is commented out and not used in this test. If it's no longer needed for the test logic, consider removing it to keep the test clean.🧹 Suggested cleanup
const graphRef = ref(mockGraph) as Ref<LGraph | null> const graphManager = useMinimapGraph(graphRef, onGraphChangedMock) - // Store original callbacks for comparison - // const originalCallbacks = { - // onNodeAdded: mockGraph.onNodeAdded, - // onNodeRemoved: mockGraph.onNodeRemoved, - // onConnectionChange: mockGraph.onConnectionChange - // } - graphManager.setupEventListeners()
christian-byrne
approved these changes
Jan 27, 2026
| throw new Error(`Setting ${setting.id} must have a unique ID.`) | ||
| // Setting already registered - skip to allow component remounting | ||
| // TODO: Add store reset methods to bootstrapStore and settingStore, then | ||
| // replace window.location.reload() with router.push() in SidebarLogoutIcon.vue |
Contributor
There was a problem hiding this comment.
Should we add this aspect to the comment/todo? Or make an Issue?
This was referenced Jan 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refactors bootstrap and lifecycle management to parallelize initialization, use Vue best practices, and fix a logout state bug.
Changes
Bootstrap Store (
bootstrapStore.ts)useAsyncStateGraphCanvas Refactoring
useGlobalLitegraph,useCopy,usePaste, etc.) to script setup level for earlier initializationwatchandwheneverdeclarations outsideonMounted(Vue best practice)until()from VueUse to await bootstrap store readiness instead of direct async callsGraphView Simplification
addEventListener/removeEventListenerwithuseEventListenersetIntervalwithuseIntervalFnfor automatic cleanupBug Fix
Using
router.push()for logout causedisSettingsReadyto persist astrue, making new users inherit the previous user's cached settings. Reverted towindow.location.reload()with TODOs for future store reset implementation.Testing