From 3a3108630de964a79ce77354fd9dec247891888f Mon Sep 17 00:00:00 2001 From: Yuchao Date: Tue, 30 Jul 2024 11:44:47 +1000 Subject: [PATCH] fix(VNumberInput): prevent NaN & handle js number quirks (#20211) fixes #19798 fixes #20171 Co-authored-by: Kael --- .../src/pages/en/components/number-inputs.md | 10 ++ .../src/labs/VNumberInput/VNumberInput.tsx | 113 +++++++++++------- .../__tests__/VNumberInput.spec.cy.tsx | 36 +++++- 3 files changed, 117 insertions(+), 42 deletions(-) diff --git a/packages/docs/src/pages/en/components/number-inputs.md b/packages/docs/src/pages/en/components/number-inputs.md index 74e0ea7437f..823a6201340 100644 --- a/packages/docs/src/pages/en/components/number-inputs.md +++ b/packages/docs/src/pages/en/components/number-inputs.md @@ -56,6 +56,16 @@ Here we display a list of settings that could be applied within an application. +## Caveats + +::: warning +**v-number-input** is designed for simple numeric input usage. It has limitations with very long integers and highly precise decimal arithmetic due to JavaScript number precision issues: + +- For integers, **v-model** is restricted within [Number.MIN_SAFE_INTEGER](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MIN_SAFE_INTEGER) and [Number.MAX_SAFE_INTEGER](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER) to ensure precision is not lost. + +- To cope with JavaScript floating-point issues (e.g. 0.1 + 0.2 === 0.30000000000000004), Vuetify's internal logic uses **toFixed()** with the maximum number of decimal places between v-model and step. If accurate arbitrary-precision decimal arithmetic is required, consider working with strings using [decimal.js](https://github.com/MikeMcl/decimal.js) and [v-text-field](/components/text-fields) instead. +::: + ## Guide The `v-number-input` component is built upon the `v-field` and `v-input` components. It is used as a replacement for ``, accepting numeric values from the user. diff --git a/packages/vuetify/src/labs/VNumberInput/VNumberInput.tsx b/packages/vuetify/src/labs/VNumberInput/VNumberInput.tsx index 75d77da5ef7..0c1b06e213b 100644 --- a/packages/vuetify/src/labs/VNumberInput/VNumberInput.tsx +++ b/packages/vuetify/src/labs/VNumberInput/VNumberInput.tsx @@ -12,7 +12,7 @@ import { useForm } from '@/composables/form' import { useProxiedModel } from '@/composables/proxiedModel' // Utilities -import { computed, watchEffect } from 'vue' +import { computed, nextTick, onMounted, ref } from 'vue' import { clamp, genericComponent, getDecimals, omit, propsFactory, useRender } from '@/util' // Types @@ -37,20 +37,24 @@ const makeVNumberInputProps = propsFactory({ }, inset: Boolean, hideInput: Boolean, + modelValue: { + type: Number as PropType, + default: null, + }, min: { type: Number, - default: -Infinity, + default: Number.MIN_SAFE_INTEGER, }, max: { type: Number, - default: Infinity, + default: Number.MAX_SAFE_INTEGER, }, step: { type: Number, default: 1, }, - ...omit(makeVTextFieldProps(), ['appendInnerIcon', 'prependInnerIcon']), + ...omit(makeVTextFieldProps({}), ['appendInnerIcon', 'modelValue', 'prependInnerIcon']), }, 'VNumberInput') export const VNumberInput = genericComponent()({ @@ -64,11 +68,20 @@ export const VNumberInput = genericComponent()({ 'update:modelValue': (val: number) => true, }, - setup (props, { attrs, emit, slots }) { - const model = useProxiedModel(props, 'modelValue') + setup (props, { slots }) { + const _model = useProxiedModel(props, 'modelValue') + + const model = computed({ + get: () => _model.value, + set (val) { + if (typeof val !== 'string') _model.value = val + }, + }) + + const vTextFieldRef = ref() const stepDecimals = computed(() => getDecimals(props.step)) - const modelDecimals = computed(() => model.value != null ? getDecimals(model.value) : 0) + const modelDecimals = computed(() => typeof model.value === 'number' ? getDecimals(model.value) : 0) const form = useForm() const controlsDisabled = computed(() => ( @@ -77,20 +90,11 @@ export const VNumberInput = genericComponent()({ const canIncrease = computed(() => { if (controlsDisabled.value) return false - if (model.value == null) return true - return model.value + props.step <= props.max + return (model.value ?? 0) as number + props.step <= props.max }) const canDecrease = computed(() => { if (controlsDisabled.value) return false - if (model.value == null) return true - return model.value - props.step >= props.min - }) - - watchEffect(() => { - if (controlsDisabled.value) return - if (model.value != null && (model.value < props.min || model.value > props.max)) { - model.value = clamp(model.value, props.min, props.max) - } + return (model.value ?? 0) as number - props.step >= props.min }) const controlVariant = computed(() => { @@ -106,18 +110,24 @@ export const VNumberInput = genericComponent()({ const decrementSlotProps = computed(() => ({ click: onClickDown })) + onMounted(() => { + if (!props.readonly && !props.disabled) { + clampModel() + } + }) + function toggleUpDown (increment = true) { if (controlsDisabled.value) return if (model.value == null) { - model.value = 0 + model.value = clamp(0, props.min, props.max) return } const decimals = Math.max(modelDecimals.value, stepDecimals.value) if (increment) { - if (canIncrease.value) model.value = +(((model.value + props.step).toFixed(decimals))) + if (canIncrease.value) model.value = +((((model.value as number) + props.step).toFixed(decimals))) } else { - if (canDecrease.value) model.value = +(((model.value - props.step).toFixed(decimals))) + if (canDecrease.value) model.value = +((((model.value as number) - props.step).toFixed(decimals))) } } @@ -131,37 +141,56 @@ export const VNumberInput = genericComponent()({ toggleUpDown(false) } - function onKeydown (e: KeyboardEvent) { + function onBeforeinput (e: InputEvent) { + if (!e.data) return + const existingTxt = (e.target as HTMLInputElement)?.value + const selectionStart = (e.target as HTMLInputElement)?.selectionStart + const selectionEnd = (e.target as HTMLInputElement)?.selectionEnd + const potentialNewInputVal = + existingTxt + ? existingTxt.slice(0, selectionStart as number | undefined) + e.data + existingTxt.slice(selectionEnd as number | undefined) + : e.data + // Only numbers, "-", "." are allowed + // AND "-", "." are allowed only once + // AND "-" is only allowed at the start + if (!/^-?(\d+(\.\d*)?|(\.\d+)|\d*|\.)$/.test(potentialNewInputVal)) { + e.preventDefault() + } + } + + async function onKeydown (e: KeyboardEvent) { if ( ['Enter', 'ArrowLeft', 'ArrowRight', 'Backspace', 'Delete', 'Tab'].includes(e.key) || e.ctrlKey ) return - if (['ArrowDown'].includes(e.key)) { - e.preventDefault() - toggleUpDown(false) - return - } - if (['ArrowUp'].includes(e.key)) { - e.preventDefault() - toggleUpDown() - return - } - - // Only numbers, +, - & . are allowed - if (!/^[0-9\-+.]+$/.test(e.key)) { + if (['ArrowDown', 'ArrowUp'].includes(e.key)) { e.preventDefault() + clampModel() + // _model is controlled, so need to wait until props['modelValue'] is updated + await nextTick() + if (e.key === 'ArrowDown') { + toggleUpDown(false) + } else { + toggleUpDown() + } } } - function onModelUpdate (v: string) { - model.value = v ? +(v) : undefined - } - function onControlMousedown (e: MouseEvent) { e.stopPropagation() } + function clampModel () { + if (!vTextFieldRef.value) return + const inputText = vTextFieldRef.value.value + if (inputText && !isNaN(+inputText)) { + model.value = clamp(+(inputText), props.min, props.max) + } else { + model.value = null + } + } + useRender(() => { const { modelValue: _, ...textFieldProps } = VTextField.filterProps(props) @@ -277,8 +306,10 @@ export const VNumberInput = genericComponent()({ return ( { + it('should prevent NaN from arbitrary input', () => { + const scenarios = [ + { typing: '---', expected: '-' }, // "-" is only allowed once + { typing: '1-', expected: '1' }, // "-" is only at the start + { typing: '.', expected: '.' }, // "." is allowed at the start + { typing: '..', expected: '.' }, // "." is only allowed once + { typing: '1...0', expected: '1.0' }, // "." is only allowed once + { typing: '123.45.67', expected: '123.4567' }, // "." is only allowed once + { typing: 'ab-c8+.iop9', expected: '-8.9' }, // Only numbers, "-", "." are allowed to type in + ] + scenarios.forEach(({ typing, expected }) => { + cy.mount(() => ) + .get('.v-number-input input').focus().realType(typing) + .get('.v-number-input input').should('have.value', expected) + }) + }) + + it('should reset v-model to null when click:clear is triggered', () => { + const model = ref(5) + + cy.mount(() => ( + <> + + + )) + .get('.v-field__clearable .v-icon--clickable').click() + .then(() => { + expect(model.value).equal(null) + }) + }) describe('readonly', () => { it('should prevent mutation when readonly applied', () => { const value = ref(1) @@ -98,7 +132,7 @@ describe('VNumberInput', () => { class="disabled-input-2" v-model={ value4.value } min={ 0 } - max={ 10 } + max={ 10 } disabled />