mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-01-26 19:09:52 +00:00
fix large integer precision handling in Vue int widgets (#5787)
## Summary Fixed increment/decrement button lockup in number widgets when values exceed JavaScript's safe integer limit (2^53 - 1). ## Changes - **What**: Added precision-aware button disabling and user feedback to `WidgetInputNumberInput` component using [Number.isSafeInteger()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isSafeInteger) - We still need to support values greater than 2^53 because they may be in workflows. ## Review Focus JavaScript floating-point precision behavior at scale - buttons hide when arithmetic operations like `value + 1` would be unreliable due to IEEE 754 limitations. Test coverage includes edge cases (NaN, Infinity) and boundary conditions at MAX_SAFE_INTEGER. ```mermaid graph TD A[User Input] --> B{Value > 2^53?} B -->|No| C[Show Buttons] B -->|Yes| D[Hide Buttons] D --> E[Show Tooltip] E --> F[User Can Still Type] style A fill:#f9f9f9,stroke:#333,color:#000 style F fill:#f9f9f9,stroke:#333,color:#000 ``` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5787-fix-large-integer-precision-handling-in-Vue-int-widgets-27a6d73d365081d9ae00e485740cfafb) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -206,3 +206,148 @@ describe('WidgetInputNumberInput Grouping Behavior', () => {
|
||||
expect(input.value).not.toContain(',')
|
||||
})
|
||||
})
|
||||
|
||||
describe('WidgetInputNumberInput Large Integer Precision Handling', () => {
|
||||
const SAFE_INTEGER_MAX = Number.MAX_SAFE_INTEGER // 9,007,199,254,740,991
|
||||
const UNSAFE_LARGE_INTEGER = 18446744073709552000 // Example seed value that exceeds safe range
|
||||
|
||||
it('shows buttons for safe integer values', () => {
|
||||
const widget = createMockWidget(1000, 'int')
|
||||
const wrapper = mountComponent(widget, 1000)
|
||||
|
||||
const inputNumber = wrapper.findComponent(InputNumber)
|
||||
expect(inputNumber.props('showButtons')).toBe(true)
|
||||
})
|
||||
|
||||
it('shows buttons for values at safe integer limit', () => {
|
||||
const widget = createMockWidget(SAFE_INTEGER_MAX, 'int')
|
||||
const wrapper = mountComponent(widget, SAFE_INTEGER_MAX)
|
||||
|
||||
const inputNumber = wrapper.findComponent(InputNumber)
|
||||
expect(inputNumber.props('showButtons')).toBe(true)
|
||||
})
|
||||
|
||||
it('hides buttons for unsafe large integer values', () => {
|
||||
const widget = createMockWidget(UNSAFE_LARGE_INTEGER, 'int')
|
||||
const wrapper = mountComponent(widget, UNSAFE_LARGE_INTEGER)
|
||||
|
||||
const inputNumber = wrapper.findComponent(InputNumber)
|
||||
expect(inputNumber.props('showButtons')).toBe(false)
|
||||
})
|
||||
|
||||
it('hides buttons for unsafe negative integer values', () => {
|
||||
const unsafeNegative = -UNSAFE_LARGE_INTEGER
|
||||
const widget = createMockWidget(unsafeNegative, 'int')
|
||||
const wrapper = mountComponent(widget, unsafeNegative)
|
||||
|
||||
const inputNumber = wrapper.findComponent(InputNumber)
|
||||
expect(inputNumber.props('showButtons')).toBe(false)
|
||||
})
|
||||
|
||||
it('shows tooltip for disabled buttons due to precision limits', () => {
|
||||
const widget = createMockWidget(UNSAFE_LARGE_INTEGER, 'int')
|
||||
const wrapper = mountComponent(widget, UNSAFE_LARGE_INTEGER)
|
||||
|
||||
// Check that tooltip wrapper div exists
|
||||
const tooltipDiv = wrapper.find('div[v-tooltip]')
|
||||
expect(tooltipDiv.exists()).toBe(true)
|
||||
})
|
||||
|
||||
it('does not show tooltip for safe integer values', () => {
|
||||
const widget = createMockWidget(1000, 'int')
|
||||
const wrapper = mountComponent(widget, 1000)
|
||||
|
||||
// For safe values, tooltip should not be set (computed returns null)
|
||||
const tooltipDiv = wrapper.find('div')
|
||||
expect(tooltipDiv.attributes('v-tooltip')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('handles edge case of zero value', () => {
|
||||
const widget = createMockWidget(0, 'int')
|
||||
const wrapper = mountComponent(widget, 0)
|
||||
|
||||
const inputNumber = wrapper.findComponent(InputNumber)
|
||||
expect(inputNumber.props('showButtons')).toBe(true)
|
||||
})
|
||||
|
||||
it('correctly identifies safe vs unsafe integers using Number.isSafeInteger', () => {
|
||||
// Test the JavaScript behavior our component relies on
|
||||
expect(Number.isSafeInteger(SAFE_INTEGER_MAX)).toBe(true)
|
||||
expect(Number.isSafeInteger(SAFE_INTEGER_MAX + 1)).toBe(false)
|
||||
expect(Number.isSafeInteger(UNSAFE_LARGE_INTEGER)).toBe(false)
|
||||
expect(Number.isSafeInteger(-SAFE_INTEGER_MAX)).toBe(true)
|
||||
expect(Number.isSafeInteger(-SAFE_INTEGER_MAX - 1)).toBe(false)
|
||||
})
|
||||
|
||||
it('maintains readonly behavior even for unsafe values', () => {
|
||||
const widget = createMockWidget(UNSAFE_LARGE_INTEGER, 'int')
|
||||
const wrapper = mountComponent(widget, UNSAFE_LARGE_INTEGER, true)
|
||||
|
||||
const inputNumber = wrapper.findComponent(InputNumber)
|
||||
expect(inputNumber.props('disabled')).toBe(true)
|
||||
expect(inputNumber.props('showButtons')).toBe(false) // Still hidden due to unsafe value
|
||||
})
|
||||
|
||||
it('handles floating point values correctly', () => {
|
||||
const safeFloat = 1000.5
|
||||
const widget = createMockWidget(safeFloat, 'float')
|
||||
const wrapper = mountComponent(widget, safeFloat)
|
||||
|
||||
const inputNumber = wrapper.findComponent(InputNumber)
|
||||
expect(inputNumber.props('showButtons')).toBe(true)
|
||||
})
|
||||
|
||||
it('hides buttons for unsafe floating point values', () => {
|
||||
const unsafeFloat = UNSAFE_LARGE_INTEGER + 0.5
|
||||
const widget = createMockWidget(unsafeFloat, 'float')
|
||||
const wrapper = mountComponent(widget, unsafeFloat)
|
||||
|
||||
const inputNumber = wrapper.findComponent(InputNumber)
|
||||
expect(inputNumber.props('showButtons')).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('WidgetInputNumberInput Edge Cases for Precision Handling', () => {
|
||||
it('handles null/undefined model values gracefully', () => {
|
||||
const widget = createMockWidget(0, 'int')
|
||||
// Mount with undefined as modelValue
|
||||
const wrapper = mount(WidgetInputNumberInput, {
|
||||
global: {
|
||||
plugins: [PrimeVue],
|
||||
components: { InputNumber }
|
||||
},
|
||||
props: {
|
||||
widget,
|
||||
modelValue: undefined as any
|
||||
}
|
||||
})
|
||||
|
||||
const inputNumber = wrapper.findComponent(InputNumber)
|
||||
expect(inputNumber.props('showButtons')).toBe(true) // Should default to safe behavior
|
||||
})
|
||||
|
||||
it('handles NaN values gracefully', () => {
|
||||
const widget = createMockWidget(NaN, 'int')
|
||||
const wrapper = mountComponent(widget, NaN)
|
||||
|
||||
const inputNumber = wrapper.findComponent(InputNumber)
|
||||
// NaN is not a safe integer, so buttons should be hidden
|
||||
expect(inputNumber.props('showButtons')).toBe(false)
|
||||
})
|
||||
|
||||
it('handles Infinity values', () => {
|
||||
const widget = createMockWidget(Infinity, 'int')
|
||||
const wrapper = mountComponent(widget, Infinity)
|
||||
|
||||
const inputNumber = wrapper.findComponent(InputNumber)
|
||||
expect(inputNumber.props('showButtons')).toBe(false)
|
||||
})
|
||||
|
||||
it('handles negative Infinity values', () => {
|
||||
const widget = createMockWidget(-Infinity, 'int')
|
||||
const wrapper = mountComponent(widget, -Infinity)
|
||||
|
||||
const inputNumber = wrapper.findComponent(InputNumber)
|
||||
expect(inputNumber.props('showButtons')).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
import InputNumber from 'primevue/inputnumber'
|
||||
import { computed } from 'vue'
|
||||
|
||||
import { useNumberWidgetValue } from '@/composables/graph/useWidgetValue'
|
||||
import type { SimplifiedWidget } from '@/types/simplifiedWidget'
|
||||
import { cn } from '@/utils/tailwindUtil'
|
||||
import {
|
||||
@@ -14,10 +15,19 @@ import WidgetLayoutField from './layout/WidgetLayoutField.vue'
|
||||
|
||||
const props = defineProps<{
|
||||
widget: SimplifiedWidget<number>
|
||||
modelValue: number
|
||||
readonly?: boolean
|
||||
}>()
|
||||
|
||||
const modelValue = defineModel<number>({ default: 0 })
|
||||
const emit = defineEmits<{
|
||||
'update:modelValue': [value: number]
|
||||
}>()
|
||||
|
||||
const { localValue, onChange } = useNumberWidgetValue(
|
||||
props.widget,
|
||||
props.modelValue,
|
||||
emit
|
||||
)
|
||||
|
||||
const filteredProps = computed(() =>
|
||||
filterWidgetProps(props.widget.options, INPUT_EXCLUDED_PROPS)
|
||||
@@ -53,34 +63,52 @@ const stepValue = computed(() => {
|
||||
const useGrouping = computed(() => {
|
||||
return props.widget.options?.useGrouping === true
|
||||
})
|
||||
|
||||
// Check if increment/decrement buttons should be disabled due to precision limits
|
||||
const buttonsDisabled = computed(() => {
|
||||
const currentValue = localValue.value || 0
|
||||
return !Number.isSafeInteger(currentValue)
|
||||
})
|
||||
|
||||
// Tooltip message for disabled buttons
|
||||
const buttonTooltip = computed(() => {
|
||||
if (props.readonly) return null
|
||||
if (buttonsDisabled.value) {
|
||||
return 'Increment/decrement disabled: value exceeds JavaScript precision limit (±2^53)'
|
||||
}
|
||||
return null
|
||||
})
|
||||
</script>
|
||||
|
||||
<template>
|
||||
<WidgetLayoutField :widget>
|
||||
<InputNumber
|
||||
v-model="modelValue"
|
||||
v-bind="filteredProps"
|
||||
show-buttons
|
||||
button-layout="horizontal"
|
||||
size="small"
|
||||
:disabled="readonly"
|
||||
:step="stepValue"
|
||||
:use-grouping="useGrouping"
|
||||
:class="cn(WidgetInputBaseClass, 'w-full text-xs')"
|
||||
:pt="{
|
||||
incrementButton:
|
||||
'!rounded-r-lg bg-transparent border-none hover:bg-zinc-500/30 active:bg-zinc-500/40',
|
||||
decrementButton:
|
||||
'!rounded-l-lg bg-transparent border-none hover:bg-zinc-500/30 active:bg-zinc-500/40'
|
||||
}"
|
||||
>
|
||||
<template #incrementicon>
|
||||
<span class="pi pi-plus text-sm" />
|
||||
</template>
|
||||
<template #decrementicon>
|
||||
<span class="pi pi-minus text-sm" />
|
||||
</template>
|
||||
</InputNumber>
|
||||
<div v-tooltip="buttonTooltip">
|
||||
<InputNumber
|
||||
v-model="localValue"
|
||||
v-bind="filteredProps"
|
||||
:show-buttons="!buttonsDisabled"
|
||||
button-layout="horizontal"
|
||||
size="small"
|
||||
:disabled="readonly"
|
||||
:step="stepValue"
|
||||
:use-grouping="useGrouping"
|
||||
:class="cn(WidgetInputBaseClass, 'w-full text-xs')"
|
||||
:pt="{
|
||||
incrementButton:
|
||||
'!rounded-r-lg bg-transparent border-none hover:bg-zinc-500/30 active:bg-zinc-500/40',
|
||||
decrementButton:
|
||||
'!rounded-l-lg bg-transparent border-none hover:bg-zinc-500/30 active:bg-zinc-500/40'
|
||||
}"
|
||||
@update:model-value="onChange"
|
||||
>
|
||||
<template #incrementicon>
|
||||
<span class="pi pi-plus text-sm" />
|
||||
</template>
|
||||
<template #decrementicon>
|
||||
<span class="pi pi-minus text-sm" />
|
||||
</template>
|
||||
</InputNumber>
|
||||
</div>
|
||||
</WidgetLayoutField>
|
||||
</template>
|
||||
|
||||
|
||||
Reference in New Issue
Block a user