Skip to content

refactor: rename isSubscribed for better expressiveness and avoid initializing cloud modules on local #7127

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.

Sign up for GitHub

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

Closed
christian-byrne wants to merge 8 commits into from
Closed

refactor: rename isSubscribed for better expressiveness and avoid initializing cloud modules on local #7127

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9d88919
rename isSubscribed and dont import cloud on local
christian-byrne Dec 3, 2025
afaceb5
dont import cloud in core commands module
christian-byrne Dec 4, 2025
cd15c27
improve test
christian-byrne Dec 6, 2025
5fd6aa6
don't use compound name, use isSubscriptionRequirementMet
christian-byrne Dec 6, 2025
6fb677f
chore: keep legacy subscription alias
christian-byrne Dec 6, 2025
1ebdab9
fix: use subscription requirement flag in cloud remote config
christian-byrne Dec 6, 2025
8ce0c9e
fix: skip subscription check when not required
christian-byrne Dec 6, 2025
b8f563d
refactor: split subscription popover section
christian-byrne Dec 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/components/actionbar/ComfyRunButton/CloudRunButtonWrapper.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<template>
<component
:is="currentButton"
:key="isActiveSubscription ? 'queue' : 'subscribe'"
:key="isSubscriptionRequirementMet ? 'queue' : 'subscribe'"
/>
</template>
<script setup lang="ts">
Expand All @@ -11,9 +11,9 @@ import ComfyQueueButton from '@/components/actionbar/ComfyRunButton/ComfyQueueBu
import SubscribeToRunButton from '@/platform/cloud/subscription/components/SubscribeToRun.vue'
import { useSubscription } from '@/platform/cloud/subscription/composables/useSubscription'

const { isActiveSubscription } = useSubscription()
const { isSubscriptionRequirementMet } = useSubscription()

const currentButton = computed(() =>
isActiveSubscription.value ? ComfyQueueButton : SubscribeToRunButton
isSubscriptionRequirementMet.value ? ComfyQueueButton : SubscribeToRunButton
)
</script>
9 changes: 7 additions & 2 deletions src/components/dialog/content/setting/CreditsPanel.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<UserCredit text-class="text-3xl font-bold" />
<Skeleton v-if="loading" width="2rem" height="2rem" />
<Button
v-else-if="isActiveSubscription"
v-else-if="isSubscriptionRequirementMet"
:label="$t('credits.purchaseCredits')"
:loading="loading"
@click="handlePurchaseCreditsClick"
Expand Down Expand Up @@ -125,6 +125,7 @@ import UsageLogsTable from '@/components/dialog/content/setting/UsageLogsTable.v
import { useFirebaseAuthActions } from '@/composables/auth/useFirebaseAuthActions'
import { useExternalLink } from '@/composables/useExternalLink'
import { useSubscription } from '@/platform/cloud/subscription/composables/useSubscription'
import { isCloud } from '@/platform/distribution/types'
import { useTelemetry } from '@/platform/telemetry'
import { useDialogService } from '@/services/dialogService'
import { useCommandStore } from '@/stores/commandStore'
Expand All @@ -144,7 +145,11 @@ const authStore = useFirebaseAuthStore()
const authActions = useFirebaseAuthActions()
const commandStore = useCommandStore()
const telemetry = useTelemetry()
const { isActiveSubscription } = useSubscription()
const subscription = isCloud ? useSubscription() : null
const isSubscriptionRequirementMet = computed(() => {
if (!isCloud) return true
return subscription?.isSubscriptionRequirementMet.value ?? false
})
const loading = computed(() => authStore.loading)
const balanceLoading = computed(() => authStore.isFetchingBalance)

Expand Down
2 changes: 1 addition & 1 deletion src/components/topbar/CurrentUserPopover.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ vi.mock('@/stores/firebaseAuthStore', () => ({
const mockFetchStatus = vi.fn().mockResolvedValue(undefined)
vi.mock('@/platform/cloud/subscription/composables/useSubscription', () => ({
useSubscription: vi.fn(() => ({
isActiveSubscription: { value: true },
isSubscriptionRequirementMet: { value: true },
fetchStatus: mockFetchStatus
}))
}))
Expand Down
14 changes: 10 additions & 4 deletions src/components/topbar/CurrentUserPopover.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@
</div>
</div>

<div v-if="isActiveSubscription" class="flex items-center justify-between">
<div
v-if="isSubscriptionRequirementMet"
class="flex items-center justify-between"
>
<div class="flex flex-col gap-1">
<UserCredit text-class="text-2xl" />
<Button
Expand Down Expand Up @@ -68,7 +71,7 @@
/>

<Button
v-if="isActiveSubscription"
v-if="isSubscriptionRequirementMet"
class="justify-start"
:label="$t(planSettingsLabel)"
icon="pi pi-receipt"
Expand All @@ -95,7 +98,7 @@
<script setup lang="ts">
import Button from 'primevue/button'
import Divider from 'primevue/divider'
import { onMounted } from 'vue'
import { computed, onMounted } from 'vue'

import UserAvatar from '@/components/common/UserAvatar.vue'
import UserCredit from '@/components/common/UserCredit.vue'
Expand All @@ -122,7 +125,10 @@ const { userDisplayName, userEmail, userPhotoUrl, handleSignOut } =
useCurrentUser()
const authActions = useFirebaseAuthActions()
const dialogService = useDialogService()
const { isActiveSubscription, fetchStatus } = useSubscription()
const subscription = isCloud ? useSubscription() : null
const isSubscriptionRequirementMet =
subscription?.isSubscriptionRequirementMet ?? computed(() => true)
const fetchStatus = subscription?.fetchStatus ?? (async () => {})

const handleOpenUserSettings = () => {
dialogService.showSettingsDialog('user')
Expand Down
10 changes: 7 additions & 3 deletions src/composables/auth/useFirebaseAuthActions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { useErrorHandling } from '@/composables/useErrorHandling'
import type { ErrorRecoveryStrategy } from '@/composables/useErrorHandling'
import { t } from '@/i18n'
import { isCloud } from '@/platform/distribution/types'
import { useSubscription } from '@/platform/cloud/subscription/composables/useSubscription'
import { useTelemetry } from '@/platform/telemetry'
import { useToastStore } from '@/platform/updates/common/toastStore'
import { useDialogService } from '@/services/dialogService'
Expand Down Expand Up @@ -82,8 +81,13 @@ export const useFirebaseAuthActions = () => {
)

const purchaseCredits = wrapWithErrorHandlingAsync(async (amount: number) => {
const { isActiveSubscription } = useSubscription()
if (!isActiveSubscription.value) return
if (isCloud) {
const { useSubscription } = await import(
'@/platform/cloud/subscription/composables/useSubscription'
)
const { isSubscriptionRequirementMet } = useSubscription()
if (!isSubscriptionRequirementMet.value) return
}

const response = await authStore.initiateCreditPurchase({
amount_micros: usdToMicros(amount),
Expand Down
23 changes: 16 additions & 7 deletions src/composables/useCoreCommands.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { computed } from 'vue'

import { useCurrentUser } from '@/composables/auth/useCurrentUser'
import { useFirebaseAuthActions } from '@/composables/auth/useFirebaseAuthActions'
import { useSelectedLiteGraphItems } from '@/composables/canvas/useSelectedLiteGraphItems'
Expand Down Expand Up @@ -46,6 +48,7 @@ import { useQueueSettingsStore, useQueueStore } from '@/stores/queueStore'
import { useSubgraphNavigationStore } from '@/stores/subgraphNavigationStore'
import { useSubgraphStore } from '@/stores/subgraphStore'
import { useBottomPanelStore } from '@/stores/workspace/bottomPanelStore'
import { isCloud } from '@/platform/distribution/types'
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Import placement breaks alphabetical grouping.

The isCloud import is inserted between unrelated imports, breaking the alphabetical ordering within the import group. This should be placed with other platform imports or sorted according to the project's import conventions.

-import { useBottomPanelStore } from '@/stores/workspace/bottomPanelStore'
-import { isCloud } from '@/platform/distribution/types'
-import { useColorPaletteStore } from '@/stores/workspace/colorPaletteStore'
+import { isCloud } from '@/platform/distribution/types'
+import { useBottomPanelStore } from '@/stores/workspace/bottomPanelStore'
+import { useColorPaletteStore } from '@/stores/workspace/colorPaletteStore'

Note: Run pnpm format to auto-sort imports per the coding guidelines.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/composables/useCoreCommands.ts around line 52, the import "isCloud" was
inserted between unrelated imports and breaks the file's alphabetical/grouping
convention; move the import into the existing platform/distribution import block
(or next to other platform imports) and reorder imports alphabetically per
project conventions, then run "pnpm format" to auto-sort and apply formatting.

import { useColorPaletteStore } from '@/stores/workspace/colorPaletteStore'
import { useRightSidePanelStore } from '@/stores/workspace/rightSidePanelStore'
import { useSearchBoxStore } from '@/stores/workspace/searchBoxStore'
Expand All @@ -63,7 +66,8 @@ import { ManagerTab } from '@/workbench/extensions/manager/types/comfyManagerTyp

import { useWorkflowTemplateSelectorDialog } from './useWorkflowTemplateSelectorDialog'

const { isActiveSubscription, showSubscriptionDialog } = useSubscription()
const defaultSubscriptionState = computed(() => true)
const noop = () => {}

const moveSelectedNodesVersionAdded = '1.22.2'

Expand All @@ -85,6 +89,11 @@ export function useCoreCommands(): ComfyCommand[] {
useSelectedLiteGraphItems()
const getTracker = () => workflowStore.activeWorkflow?.changeTracker

const subscription = isCloud ? useSubscription() : null
const subscriptionState =
subscription?.isSubscriptionRequirementMet ?? defaultSubscriptionState
const subscriptionDialog = subscription?.showSubscriptionDialog ?? noop

const moveSelectedNodes = (
positionUpdater: (pos: Point, gridSize: number) => Point
) => {
Expand Down Expand Up @@ -475,8 +484,8 @@ export function useCoreCommands(): ComfyCommand[] {
trigger_source?: ExecutionTriggerSource
}) => {
useTelemetry()?.trackRunButton(metadata)
if (!isActiveSubscription.value) {
showSubscriptionDialog()
if (!subscriptionState.value) {
subscriptionDialog()
return
}

Expand All @@ -498,8 +507,8 @@ export function useCoreCommands(): ComfyCommand[] {
trigger_source?: ExecutionTriggerSource
}) => {
useTelemetry()?.trackRunButton(metadata)
if (!isActiveSubscription.value) {
showSubscriptionDialog()
if (!subscriptionState.value) {
subscriptionDialog()
return
}

Expand All @@ -520,8 +529,8 @@ export function useCoreCommands(): ComfyCommand[] {
trigger_source?: ExecutionTriggerSource
}) => {
useTelemetry()?.trackRunButton(metadata)
if (!isActiveSubscription.value) {
showSubscriptionDialog()
if (!subscriptionState.value) {
subscriptionDialog()
return
}

Expand Down
4 changes: 2 additions & 2 deletions src/extensions/core/cloudRemoteConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ useExtensionService().registerExtension({

setup: async () => {
const { isLoggedIn } = useCurrentUser()
const { isActiveSubscription } = useSubscription()
const { isSubscriptionRequirementMet } = useSubscription()

watchDebounced(
[isLoggedIn, isActiveSubscription],
[isLoggedIn, isSubscriptionRequirementMet],
() => {
if (!isLoggedIn.value) return
void refreshRemoteConfig()
Expand Down
5 changes: 3 additions & 2 deletions src/platform/cloud/subscription/components/SubscribeButton.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ const emit = defineEmits<{
subscribed: []
}>()

const { subscribe, isActiveSubscription, fetchStatus } = useSubscription()
const { subscribe, isSubscriptionRequirementMet, fetchStatus } =
useSubscription()
const telemetry = useTelemetry()

const isLoading = ref(false)
Expand All @@ -76,7 +77,7 @@ const startPollingSubscriptionStatus = () => {

await fetchStatus()

if (isActiveSubscription.value) {
if (isSubscriptionRequirementMet.value) {
stopPolling()
telemetry?.trackMonthlySubscriptionSucceeded()
emit('subscribed')
Expand Down
10 changes: 5 additions & 5 deletions src/platform/cloud/subscription/components/SubscriptionPanel.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<div class="flex items-baseline gap-2">
<span class="text-2xl font-inter font-semibold leading-tight">
{{
isActiveSubscription
isSubscriptionRequirementMet
? $t('subscription.title')
: $t('subscription.titleUnsubscribed')
}}
Expand All @@ -27,7 +27,7 @@
}}</span>
</div>
<div
v-if="isActiveSubscription"
v-if="isSubscriptionRequirementMet"
class="text-sm text-text-secondary"
>
<template v-if="isCancelled">
Expand All @@ -47,7 +47,7 @@
</div>
</div>
<Button
v-if="isActiveSubscription"
v-if="isSubscriptionRequirementMet"
:label="$t('subscription.manageSubscription')"
severity="secondary"
class="text-xs bg-interface-menu-component-surface-selected"
Expand Down Expand Up @@ -196,7 +196,7 @@
{{ $t('subscription.viewUsageHistory') }}
</a>
<Button
v-if="isActiveSubscription"
v-if="isSubscriptionRequirementMet"
:label="$t('subscription.addCredits')"
severity="secondary"
class="p-2 min-h-8 bg-interface-menu-component-surface-selected"
Expand Down Expand Up @@ -320,7 +320,7 @@ import { cn } from '@/utils/tailwindUtil'
const { buildDocsUrl } = useExternalLink()

const {
isActiveSubscription,
isSubscriptionRequirementMet,
isCancelled,
formattedRenewalDate,
formattedEndDate,
Expand Down
8 changes: 4 additions & 4 deletions src/platform/cloud/subscription/composables/useSubscription.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function useSubscriptionInternal() {
const subscriptionStatus = ref<CloudSubscriptionStatusResponse | null>(null)
const telemetry = useTelemetry()

const isSubscribedOrIsNotCloud = computed(() => {
const isSubscriptionRequirementMet = computed(() => {
if (!isCloud || !window.__CONFIG__?.subscription_required) return true

return subscriptionStatus.value?.is_active ?? false
Expand Down Expand Up @@ -111,7 +111,7 @@ function useSubscriptionInternal() {
const { startCancellationWatcher, stopCancellationWatcher } =
useSubscriptionCancellationWatcher({
fetchStatus,
isActiveSubscription: isSubscribedOrIsNotCloud,
isSubscriptionRequirementMet,
subscriptionStatus,
telemetry,
shouldWatchCancellation
Expand All @@ -125,7 +125,7 @@ function useSubscriptionInternal() {
const requireActiveSubscription = async (): Promise<void> => {
await fetchSubscriptionStatus()

if (!isSubscribedOrIsNotCloud.value) {
if (!isSubscriptionRequirementMet.value) {
showSubscriptionDialog()
}
}
Expand Down Expand Up @@ -223,7 +223,7 @@ function useSubscriptionInternal() {

return {
// State
isActiveSubscription: isSubscribedOrIsNotCloud,
isSubscriptionRequirementMet,
isCancelled,
formattedRenewalDate,
formattedEndDate,
Expand Down
4 changes: 2 additions & 2 deletions src/platform/cloud/subscription/composables/useSubscriptionActions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ export function useSubscriptionActions() {
void handleRefresh()
})

const handleAddApiCredits = () => {
dialogService.showTopUpCreditsDialog()
const handleAddApiCredits = async () => {
await dialogService.showTopUpCreditsDialog()
}

const handleMessageSupport = async () => {
Expand Down
6 changes: 3 additions & 3 deletions src/platform/cloud/subscription/composables/useSubscriptionCancellationWatcher.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ const CANCELLATION_BACKOFF_MULTIPLIER = 3 // 5s, 15s, 45s, 135s intervals

type CancellationWatcherOptions = {
fetchStatus: () => Promise<CloudSubscriptionStatusResponse | null | void>
isActiveSubscription: ComputedRef<boolean>
isSubscriptionRequirementMet: ComputedRef<boolean>
subscriptionStatus: Ref<CloudSubscriptionStatusResponse | null>
telemetry: Pick<TelemetryProvider, 'trackMonthlySubscriptionCancelled'> | null
shouldWatchCancellation: () => boolean
}

export function useSubscriptionCancellationWatcher({
fetchStatus,
isActiveSubscription,
isSubscriptionRequirementMet,
subscriptionStatus,
telemetry,
shouldWatchCancellation
Expand Down Expand Up @@ -73,7 +73,7 @@ export function useSubscriptionCancellationWatcher({
try {
await fetchStatus()

if (!isActiveSubscription.value) {
if (!isSubscriptionRequirementMet.value) {
if (!cancellationTracked.value) {
cancellationTracked.value = true
try {
Expand Down
10 changes: 8 additions & 2 deletions src/scripts/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -676,11 +676,17 @@ export class ComfyApp {
'Payment Required: Please add credits to your account to use this node.'
)
) {
const { isActiveSubscription } = useSubscription()
if (isActiveSubscription.value) {
if (!isCloud) {
useDialogService().showTopUpCreditsDialog({
isInsufficientCredits: true
})
} else {
const { isSubscriptionRequirementMet } = useSubscription()
if (isSubscriptionRequirementMet.value) {
useDialogService().showTopUpCreditsDialog({
isInsufficientCredits: true
})
}
}
} else {
useDialogService().showExecutionErrorDialog(detail)
Expand Down
12 changes: 8 additions & 4 deletions src/services/dialogService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import SettingDialogHeader from '@/components/dialog/header/SettingDialogHeader.
import { t } from '@/i18n'
import { useTelemetry } from '@/platform/telemetry'
import { isCloud } from '@/platform/distribution/types'
import { useSubscription } from '@/platform/cloud/subscription/composables/useSubscription'
import SettingDialogContent from '@/platform/settings/components/SettingDialogContent.vue'
import type { ExecutionErrorWsMessage } from '@/schemas/apiSchema'
import { useDialogStore } from '@/stores/dialogStore'
Expand Down Expand Up @@ -377,11 +376,16 @@ export const useDialogService = () => {
})
}

function showTopUpCreditsDialog(options?: {
async function showTopUpCreditsDialog(options?: {
isInsufficientCredits?: boolean
}) {
const { isActiveSubscription } = useSubscription()
if (!isActiveSubscription.value) return
if (isCloud) {
const { useSubscription } = await import(
'@/platform/cloud/subscription/composables/useSubscription'
)
const { isSubscriptionRequirementMet } = useSubscription()
if (!isSubscriptionRequirementMet.value) return
}
Comment on lines +379 to +388
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

rg -n "showTopUpCreditsDialog" src

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 999

🏁 Script executed:

# Check app.ts context around lines 680, 686
sed -n '675,695n' src/scripts/app.ts

# Check CurrentUserPopover.vue context around line 151
sed -n '145,160n' src/components/topbar/CurrentUserPopover.vue

# Check CreditsPanel.vue context around line 176
sed -n '170,185n' src/components/dialog/content/setting/CreditsPanel.vue

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 52

🏁 Script executed:

# Check app.ts around lines 680, 686
cat -n src/scripts/app.ts | sed -n '675,695p'

# Check CurrentUserPopover.vue around line 151
cat -n src/components/topbar/CurrentUserPopover.vue | sed -n '145,160p'

# Check CreditsPanel.vue around line 176
cat -n src/components/dialog/content/setting/CreditsPanel.vue | sed -n '170,185p'

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 1916

🏁 Script executed:

# Read the full showTopUpCreditsDialog function from dialogService.ts
sed -n '379,430p' src/services/dialogService.ts | cat -n

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 2060

🏁 Script executed:

# Find dialogStore and its showDialog method
rg -n "showDialog" src/stores/dialogStore.ts -A 5 | head -40

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 442

🏁 Script executed:

# Check the full showDialog implementation
sed -n '212,245p' src/stores/dialogStore.ts | cat -n

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 1404

Add await to showTopUpCreditsDialog calls to ensure subscription requirement check completes before dialog opens

The function is now async with a subscription requirement check in the cloud path (lines 384-387), but four call sites invoke it without await: lines 680 and 686 in src/scripts/app.ts, line 151 in src/components/topbar/CurrentUserPopover.vue, and line 176 in src/components/dialog/content/setting/CreditsPanel.vue. Without awaiting, the subscription requirement check may not complete before the dialog opens, bypassing the intended gate for cloud users. Update these fire-and-forget calls to await dialogService.showTopUpCreditsDialog() (line 40 in src/platform/cloud/subscription/composables/useSubscriptionActions.ts already does this correctly).


return dialogStore.showDialog({
key: 'top-up-credits',
Expand Down
Loading
Loading