From 9da2a5b6de93da461e5ef6700f85ec0d637abc6b Mon Sep 17 00:00:00 2001
From: bymyself <cbyrne@comfy.org>
Date: Wed, 6 Aug 2025 13:43:42 -0700
Subject: [PATCH] [refactor] replace manual semver operations with semver
 package

Replace custom compareVersions and isSemVer functions with the robust semver package to handle version comparisons more reliably. This addresses edge cases and follows industry standards for semantic version handling.
---
 .../content/MissingCoreNodesMessage.vue       |   7 +-
 .../content/manager/PackVersionBadge.vue      |   6 +-
 .../manager/PackVersionSelectorPopover.vue    |   4 +-
 .../nodePack/usePackUpdateStatus.ts           |   9 +-
 src/stores/releaseStore.ts                    |  16 ++-
 src/stores/settingStore.ts                    |  25 ++++-
 src/utils/formatUtil.ts                       |  33 ------
 tests-ui/tests/store/releaseStore.test.ts     | 103 +++++++++---------
 8 files changed, 99 insertions(+), 104 deletions(-)

diff --git a/src/components/dialog/content/MissingCoreNodesMessage.vue b/src/components/dialog/content/MissingCoreNodesMessage.vue
index 036f088b3..5f06278cc 100644
--- a/src/components/dialog/content/MissingCoreNodesMessage.vue
+++ b/src/components/dialog/content/MissingCoreNodesMessage.vue
@@ -44,11 +44,11 @@
 <script setup lang="ts">
 import { whenever } from '@vueuse/core'
 import Message from 'primevue/message'
+import * as semver from 'semver'
 import { computed, ref } from 'vue'
 
 import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
 import { useSystemStatsStore } from '@/stores/systemStatsStore'
-import { compareVersions } from '@/utils/formatUtil'
 
 const props = defineProps<{
   missingCoreNodes: Record<string, LGraphNode[]>
@@ -78,7 +78,10 @@ whenever(
 const sortedMissingCoreNodes = computed(() => {
   return Object.entries(props.missingCoreNodes).sort(([a], [b]) => {
     // Sort by version in descending order (newest first)
-    return compareVersions(b, a) // Reversed for descending order
+    const versionA = semver.coerce(a)
+    const versionB = semver.coerce(b)
+    if (!versionA || !versionB) return 0
+    return semver.rcompare(versionA, versionB)
   })
 })
 
diff --git a/src/components/dialog/content/manager/PackVersionBadge.vue b/src/components/dialog/content/manager/PackVersionBadge.vue
index 5a6ee32e2..9765a34e3 100644
--- a/src/components/dialog/content/manager/PackVersionBadge.vue
+++ b/src/components/dialog/content/manager/PackVersionBadge.vue
@@ -37,6 +37,7 @@
 
 <script setup lang="ts">
 import Popover from 'primevue/popover'
+import * as semver from 'semver'
 import { computed, ref, watch } from 'vue'
 
 import PackVersionSelectorPopover from '@/components/dialog/content/manager/PackVersionSelectorPopover.vue'
@@ -44,7 +45,6 @@ import { usePackUpdateStatus } from '@/composables/nodePack/usePackUpdateStatus'
 import { useComfyManagerStore } from '@/stores/comfyManagerStore'
 import { SelectedVersion } from '@/types/comfyManagerTypes'
 import { components } from '@/types/comfyRegistryTypes'
-import { isSemVer } from '@/utils/formatUtil'
 
 const TRUNCATED_HASH_LENGTH = 7
 
@@ -71,7 +71,9 @@ const installedVersion = computed(() => {
     SelectedVersion.NIGHTLY
 
   // If Git hash, truncate to 7 characters
-  return isSemVer(version) ? version : version.slice(0, TRUNCATED_HASH_LENGTH)
+  return semver.valid(version)
+    ? version
+    : version.slice(0, TRUNCATED_HASH_LENGTH)
 })
 
 const toggleVersionSelector = (event: Event) => {
diff --git a/src/components/dialog/content/manager/PackVersionSelectorPopover.vue b/src/components/dialog/content/manager/PackVersionSelectorPopover.vue
index 020b3e352..0a8d8785d 100644
--- a/src/components/dialog/content/manager/PackVersionSelectorPopover.vue
+++ b/src/components/dialog/content/manager/PackVersionSelectorPopover.vue
@@ -62,6 +62,7 @@ import { whenever } from '@vueuse/core'
 import Button from 'primevue/button'
 import Listbox from 'primevue/listbox'
 import ProgressSpinner from 'primevue/progressspinner'
+import * as semver from 'semver'
 import { onMounted, ref } from 'vue'
 import { useI18n } from 'vue-i18n'
 
@@ -75,7 +76,6 @@ import {
   SelectedVersion
 } from '@/types/comfyManagerTypes'
 import { components } from '@/types/comfyRegistryTypes'
-import { isSemVer } from '@/utils/formatUtil'
 
 const { nodePack } = defineProps<{
   nodePack: components['schemas']['Node']
@@ -97,7 +97,7 @@ onMounted(() => {
   const initialVersion = getInitialSelectedVersion() ?? SelectedVersion.LATEST
   selectedVersion.value =
     // Use NIGHTLY when version is a Git hash
-    isSemVer(initialVersion) ? initialVersion : SelectedVersion.NIGHTLY
+    semver.valid(initialVersion) ? initialVersion : SelectedVersion.NIGHTLY
 })
 
 const getInitialSelectedVersion = () => {
diff --git a/src/composables/nodePack/usePackUpdateStatus.ts b/src/composables/nodePack/usePackUpdateStatus.ts
index f8344cd2b..c24b92b9c 100644
--- a/src/composables/nodePack/usePackUpdateStatus.ts
+++ b/src/composables/nodePack/usePackUpdateStatus.ts
@@ -1,8 +1,8 @@
+import * as semver from 'semver'
 import { computed } from 'vue'
 
 import { useComfyManagerStore } from '@/stores/comfyManagerStore'
 import type { components } from '@/types/comfyRegistryTypes'
-import { compareVersions, isSemVer } from '@/utils/formatUtil'
 
 export const usePackUpdateStatus = (
   nodePack: components['schemas']['Node']
@@ -16,14 +16,17 @@ export const usePackUpdateStatus = (
   const latestVersion = computed(() => nodePack.latest_version?.version)
 
   const isNightlyPack = computed(
-    () => !!installedVersion.value && !isSemVer(installedVersion.value)
+    () => !!installedVersion.value && !semver.valid(installedVersion.value)
   )
 
   const isUpdateAvailable = computed(() => {
     if (!isInstalled.value || isNightlyPack.value || !latestVersion.value) {
       return false
     }
-    return compareVersions(latestVersion.value, installedVersion.value) > 0
+    const installedSemver = semver.coerce(installedVersion.value)
+    const latestSemver = semver.coerce(latestVersion.value)
+    if (!installedSemver || !latestSemver) return false
+    return semver.gt(latestSemver, installedSemver)
   })
 
   return {
diff --git a/src/stores/releaseStore.ts b/src/stores/releaseStore.ts
index b795fbca1..940aabf82 100644
--- a/src/stores/releaseStore.ts
+++ b/src/stores/releaseStore.ts
@@ -1,11 +1,12 @@
 import { defineStore } from 'pinia'
+import * as semver from 'semver'
 import { computed, ref } from 'vue'
 
 import { type ReleaseNote, useReleaseService } from '@/services/releaseService'
 import { useSettingStore } from '@/stores/settingStore'
 import { useSystemStatsStore } from '@/stores/systemStatsStore'
 import { isElectron } from '@/utils/envUtil'
-import { compareVersions, stringToLocale } from '@/utils/formatUtil'
+import { stringToLocale } from '@/utils/formatUtil'
 
 // Store for managing release notes
 export const useReleaseStore = defineStore('release', () => {
@@ -54,16 +55,19 @@ export const useReleaseStore = defineStore('release', () => {
   const isNewVersionAvailable = computed(
     () =>
       !!recentRelease.value &&
-      compareVersions(
-        recentRelease.value.version,
-        currentComfyUIVersion.value
-      ) > 0
+      semver.gt(
+        semver.coerce(recentRelease.value.version) || '0.0.0',
+        semver.coerce(currentComfyUIVersion.value) || '0.0.0'
+      )
   )
 
   const isLatestVersion = computed(
     () =>
       !!recentRelease.value &&
-      !compareVersions(recentRelease.value.version, currentComfyUIVersion.value)
+      semver.eq(
+        semver.coerce(recentRelease.value.version) || '0.0.0',
+        semver.coerce(currentComfyUIVersion.value) || '0.0.0'
+      )
   )
 
   const hasMediumOrHighAttention = computed(() =>
diff --git a/src/stores/settingStore.ts b/src/stores/settingStore.ts
index b3a9f5bbf..9e1512190 100644
--- a/src/stores/settingStore.ts
+++ b/src/stores/settingStore.ts
@@ -1,5 +1,6 @@
 import _ from 'lodash'
 import { defineStore } from 'pinia'
+import * as semver from 'semver'
 import { ref } from 'vue'
 
 import type { Settings } from '@/schemas/apiSchema'
@@ -7,7 +8,6 @@ import { api } from '@/scripts/api'
 import { app } from '@/scripts/app'
 import type { SettingParams } from '@/types/settingTypes'
 import type { TreeNode } from '@/types/treeExplorerTypes'
-import { compareVersions, isSemVer } from '@/utils/formatUtil'
 
 export const getSettingInfo = (setting: SettingParams) => {
   const parts = setting.category || setting.id.split('.')
@@ -132,17 +132,30 @@ export const useSettingStore = defineStore('setting', () => {
 
       if (installedVersion) {
         const sortedVersions = Object.keys(defaultsByInstallVersion).sort(
-          (a, b) => compareVersions(b, a)
+          (a, b) => {
+            const versionA = semver.coerce(a)
+            const versionB = semver.coerce(b)
+            if (!versionA || !versionB) return 0
+            return semver.rcompare(versionA, versionB)
+          }
         )
 
         for (const version of sortedVersions) {
-          // Ensure the version is in a valid format before comparing
-          if (!isSemVer(version)) {
+          if (!semver.valid(version)) {
             continue
           }
 
-          if (compareVersions(installedVersion, version) >= 0) {
-            const versionedDefault = defaultsByInstallVersion[version]
+          const installedSemver = semver.coerce(installedVersion)
+          const targetSemver = semver.coerce(version)
+          if (
+            installedSemver &&
+            targetSemver &&
+            semver.gte(installedSemver, targetSemver)
+          ) {
+            const versionedDefault =
+              defaultsByInstallVersion[
+                version as `${number}.${number}.${number}`
+              ]
             return typeof versionedDefault === 'function'
               ? versionedDefault()
               : versionedDefault
diff --git a/src/utils/formatUtil.ts b/src/utils/formatUtil.ts
index da933d3ec..eeaf8ef06 100644
--- a/src/utils/formatUtil.ts
+++ b/src/utils/formatUtil.ts
@@ -390,39 +390,6 @@ export const downloadUrlToHfRepoUrl = (url: string): string => {
   }
 }
 
-export const isSemVer = (
-  version: string
-): version is `${number}.${number}.${number}` => {
-  const regex = /^\d+\.\d+\.\d+$/
-  return regex.test(version)
-}
-
-const normalizeVersion = (version: string) =>
-  version
-    .split(/[+.-]/)
-    .map(Number)
-    .filter((part) => !Number.isNaN(part))
-
-export function compareVersions(
-  versionA: string | undefined,
-  versionB: string | undefined
-): number {
-  versionA ??= '0.0.0'
-  versionB ??= '0.0.0'
-
-  const aParts = normalizeVersion(versionA)
-  const bParts = normalizeVersion(versionB)
-
-  for (let i = 0; i < Math.max(aParts.length, bParts.length); i++) {
-    const aPart = aParts[i] ?? 0
-    const bPart = bParts[i] ?? 0
-    if (aPart < bPart) return -1
-    if (aPart > bPart) return 1
-  }
-
-  return 0
-}
-
 /**
  * Converts a currency amount to Metronome's integer representation.
  * For USD, converts to cents (multiplied by 100).
diff --git a/tests-ui/tests/store/releaseStore.test.ts b/tests-ui/tests/store/releaseStore.test.ts
index 8e49a56af..15daac7f9 100644
--- a/tests-ui/tests/store/releaseStore.test.ts
+++ b/tests-ui/tests/store/releaseStore.test.ts
@@ -4,7 +4,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { useReleaseStore } from '@/stores/releaseStore'
 
 // Mock the dependencies
-vi.mock('@/utils/formatUtil')
 vi.mock('@/utils/envUtil')
 vi.mock('@/services/releaseService')
 vi.mock('@/stores/settingStore')
@@ -107,18 +106,16 @@ describe('useReleaseStore', () => {
     })
 
     it('should show update button (shouldShowUpdateButton)', async () => {
-      const { compareVersions } = await import('@/utils/formatUtil')
-      vi.mocked(compareVersions).mockReturnValue(1) // newer version available
-
-      store.releases = [mockRelease]
+      // Mock system version to be older than release version
+      mockSystemStatsStore.systemStats.system.comfyui_version = '1.0.0'
+      store.releases = [mockRelease] // mockRelease has version 1.2.0
       expect(store.shouldShowUpdateButton).toBe(true)
     })
 
     it('should not show update button when no new version', async () => {
-      const { compareVersions } = await import('@/utils/formatUtil')
-      vi.mocked(compareVersions).mockReturnValue(-1) // current version is newer
-
-      store.releases = [mockRelease]
+      // Mock system version to be newer than release version
+      mockSystemStatsStore.systemStats.system.comfyui_version = '1.3.0'
+      store.releases = [mockRelease] // mockRelease has version 1.2.0
       expect(store.shouldShowUpdateButton).toBe(false)
     })
   })
@@ -137,8 +134,8 @@ describe('useReleaseStore', () => {
       })
 
       it('should show toast for medium/high attention releases', async () => {
-        const { compareVersions } = await import('@/utils/formatUtil')
-        vi.mocked(compareVersions).mockReturnValue(1)
+        // Mock system version to be older than release version
+        mockSystemStatsStore.systemStats.system.comfyui_version = '1.0.0'
 
         // Need multiple releases for hasMediumOrHighAttention to work
         const mediumRelease = {
@@ -152,16 +149,17 @@ describe('useReleaseStore', () => {
       })
 
       it('should show red dot for new versions', async () => {
-        const { compareVersions } = await import('@/utils/formatUtil')
-        vi.mocked(compareVersions).mockReturnValue(1)
+        // Mock system version to be older than release version
+        mockSystemStatsStore.systemStats.system.comfyui_version = '1.0.0'
+        store.releases = [mockRelease] // mockRelease has version 1.2.0
 
         expect(store.shouldShowRedDot).toBe(true)
       })
 
       it('should show popup for latest version', async () => {
+        // Mock system version to match release version
         mockSystemStatsStore.systemStats.system.comfyui_version = '1.2.0'
-        const { compareVersions } = await import('@/utils/formatUtil')
-        vi.mocked(compareVersions).mockReturnValue(0)
+        store.releases = [mockRelease] // mockRelease has version 1.2.0
 
         expect(store.shouldShowPopup).toBe(true)
       })
@@ -174,7 +172,8 @@ describe('useReleaseStore', () => {
         expect(mockReleaseService.getReleases).toHaveBeenCalledWith({
           project: 'comfyui',
           current_version: '1.0.0',
-          form_factor: 'git-windows'
+          form_factor: 'git-windows',
+          locale: 'en'
         })
       })
     })
@@ -188,23 +187,25 @@ describe('useReleaseStore', () => {
       })
 
       it('should not show toast even with new version available', async () => {
-        const { compareVersions } = await import('@/utils/formatUtil')
-        vi.mocked(compareVersions).mockReturnValue(1)
+        // Mock system version to be older than release version
+        mockSystemStatsStore.systemStats.system.comfyui_version = '1.0.0'
+        store.releases = [mockRelease] // mockRelease has version 1.2.0
 
         expect(store.shouldShowToast).toBe(false)
       })
 
       it('should not show red dot even with new version available', async () => {
-        const { compareVersions } = await import('@/utils/formatUtil')
-        vi.mocked(compareVersions).mockReturnValue(1)
+        // Mock system version to be older than release version
+        mockSystemStatsStore.systemStats.system.comfyui_version = '1.0.0'
+        store.releases = [mockRelease] // mockRelease has version 1.2.0
 
         expect(store.shouldShowRedDot).toBe(false)
       })
 
       it('should not show popup even for latest version', async () => {
+        // Mock system version to match release version
         mockSystemStatsStore.systemStats.system.comfyui_version = '1.2.0'
-        const { compareVersions } = await import('@/utils/formatUtil')
-        vi.mocked(compareVersions).mockReturnValue(0)
+        store.releases = [mockRelease] // mockRelease has version 1.2.0
 
         expect(store.shouldShowPopup).toBe(false)
       })
@@ -233,7 +234,8 @@ describe('useReleaseStore', () => {
       expect(mockReleaseService.getReleases).toHaveBeenCalledWith({
         project: 'comfyui',
         current_version: '1.0.0',
-        form_factor: 'git-windows'
+        form_factor: 'git-windows',
+        locale: 'en'
       })
       expect(store.releases).toEqual([mockRelease])
     })
@@ -247,7 +249,8 @@ describe('useReleaseStore', () => {
       expect(mockReleaseService.getReleases).toHaveBeenCalledWith({
         project: 'comfyui',
         current_version: '1.0.0',
-        form_factor: 'desktop-mac'
+        form_factor: 'desktop-mac',
+        locale: 'en'
       })
     })
 
@@ -373,8 +376,8 @@ describe('useReleaseStore', () => {
         return null
       })
 
-      const { compareVersions } = await import('@/utils/formatUtil')
-      vi.mocked(compareVersions).mockReturnValue(1)
+      // Mock system version to be older than release version
+      mockSystemStatsStore.systemStats.system.comfyui_version = '1.0.0'
 
       const mediumRelease = { ...mockRelease, attention: 'medium' as const }
       store.releases = [
@@ -387,29 +390,27 @@ describe('useReleaseStore', () => {
     })
 
     it('should show red dot for new versions', async () => {
-      const { compareVersions } = await import('@/utils/formatUtil')
-      vi.mocked(compareVersions).mockReturnValue(1)
       mockSettingStore.get.mockImplementation((key: string) => {
         if (key === 'Comfy.Notification.ShowVersionUpdates') return true
         return null
       })
 
-      store.releases = [mockRelease]
+      // Mock system version to be older than release version
+      mockSystemStatsStore.systemStats.system.comfyui_version = '1.0.0'
+      store.releases = [mockRelease] // mockRelease has version 1.2.0
 
       expect(store.shouldShowRedDot).toBe(true)
     })
 
     it('should show popup for latest version', async () => {
-      mockSystemStatsStore.systemStats.system.comfyui_version = '1.2.0' // Same as release
       mockSettingStore.get.mockImplementation((key: string) => {
         if (key === 'Comfy.Notification.ShowVersionUpdates') return true
         return null
       })
 
-      const { compareVersions } = await import('@/utils/formatUtil')
-      vi.mocked(compareVersions).mockReturnValue(0) // versions are equal (latest version)
-
-      store.releases = [mockRelease]
+      // Mock system version to match release version
+      mockSystemStatsStore.systemStats.system.comfyui_version = '1.2.0' // Same as release
+      store.releases = [mockRelease] // mockRelease has version 1.2.0
 
       expect(store.shouldShowPopup).toBe(true)
     })
@@ -465,8 +466,8 @@ describe('useReleaseStore', () => {
       })
 
       it('should show toast when conditions are met', async () => {
-        const { compareVersions } = await import('@/utils/formatUtil')
-        vi.mocked(compareVersions).mockReturnValue(1)
+        // Mock system version to be older than release version
+        mockSystemStatsStore.systemStats.system.comfyui_version = '1.0.0'
 
         // Need multiple releases for hasMediumOrHighAttention
         const mediumRelease = {
@@ -480,16 +481,17 @@ describe('useReleaseStore', () => {
       })
 
       it('should show red dot when new version available', async () => {
-        const { compareVersions } = await import('@/utils/formatUtil')
-        vi.mocked(compareVersions).mockReturnValue(1)
+        // Mock system version to be older than release version
+        mockSystemStatsStore.systemStats.system.comfyui_version = '1.0.0'
+        store.releases = [mockRelease] // mockRelease has version 1.2.0
 
         expect(store.shouldShowRedDot).toBe(true)
       })
 
       it('should show popup for latest version', async () => {
+        // Mock system version to match release version
         mockSystemStatsStore.systemStats.system.comfyui_version = '1.2.0'
-        const { compareVersions } = await import('@/utils/formatUtil')
-        vi.mocked(compareVersions).mockReturnValue(0)
+        store.releases = [mockRelease] // mockRelease has version 1.2.0
 
         expect(store.shouldShowPopup).toBe(true)
       })
@@ -502,8 +504,8 @@ describe('useReleaseStore', () => {
       })
 
       it('should NOT show toast even when all other conditions are met', async () => {
-        const { compareVersions } = await import('@/utils/formatUtil')
-        vi.mocked(compareVersions).mockReturnValue(1)
+        // Mock system version to be older than release version
+        mockSystemStatsStore.systemStats.system.comfyui_version = '1.0.0'
 
         // Set up all conditions that would normally show toast
         const mediumRelease = {
@@ -517,15 +519,16 @@ describe('useReleaseStore', () => {
       })
 
       it('should NOT show red dot even when new version available', async () => {
-        const { compareVersions } = await import('@/utils/formatUtil')
-        vi.mocked(compareVersions).mockReturnValue(1)
+        // Mock system version to be older than release version
+        mockSystemStatsStore.systemStats.system.comfyui_version = '1.0.0'
+        store.releases = [mockRelease] // mockRelease has version 1.2.0
 
         expect(store.shouldShowRedDot).toBe(false)
       })
 
       it('should NOT show toast regardless of attention level', async () => {
-        const { compareVersions } = await import('@/utils/formatUtil')
-        vi.mocked(compareVersions).mockReturnValue(1)
+        // Mock system version to be older than release version
+        mockSystemStatsStore.systemStats.system.comfyui_version = '1.0.0'
 
         // Test with high attention releases
         const highRelease = {
@@ -544,8 +547,8 @@ describe('useReleaseStore', () => {
       })
 
       it('should NOT show red dot even with high attention release', async () => {
-        const { compareVersions } = await import('@/utils/formatUtil')
-        vi.mocked(compareVersions).mockReturnValue(1)
+        // Mock system version to be older than release version
+        mockSystemStatsStore.systemStats.system.comfyui_version = '1.0.0'
 
         store.releases = [{ ...mockRelease, attention: 'high' as const }]
 
@@ -553,9 +556,9 @@ describe('useReleaseStore', () => {
       })
 
       it('should NOT show popup even for latest version', async () => {
+        // Mock system version to match release version
         mockSystemStatsStore.systemStats.system.comfyui_version = '1.2.0'
-        const { compareVersions } = await import('@/utils/formatUtil')
-        vi.mocked(compareVersions).mockReturnValue(0)
+        store.releases = [mockRelease] // mockRelease has version 1.2.0
 
         expect(store.shouldShowPopup).toBe(false)
       })