Skip to content
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.

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

Fix: [Numberinput] Validation may not work with Vue 3.4.28 or higher #429 #433

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

kikuomax
Copy link
Collaborator

@kikuomax kikuomax commented Feb 3, 2025

Proposed Changes

  • Fixes the issues that Numberinput may have accepted an invalid user input without a validation error on Vue 3.4.28 or higher.
    • To work around an upstream issue introduced in Vue 3.4.28, the underlying Input component uses v-model instead of the combination of the :value binding, and @input and @change event handlers. See "Background" for more details.
    • Please note that this PR will cause type errors regarding the v-model bindings in <textarea>. However, the type errors will be fixed if we upgrade Vue to 3.4.6 or higher.
  • More comprehensively tests the lazy prop of Input.

Background

Numberinput should reject numbers which are not equal to the basis for stepping, and the basis is derived from the value attribute unless the min is given. Before Vue 3.4.28, updating the value prop did not affect the value attribute, however, Vue 3.4.28 or higher introduced a tweak that updates the value attribute when the value prop is
changed
. Since the value attribute which defines the basis moves, the validation of Numberinput can be messed up. The problematic tweak is not applied to v-model which the underlying Input did not use.

The reason why Input did not use v-model was to deal with the lazy prop. While v-model needs an optional .lazy modifier if the lazy prop is true, unfortunately, we cannot dynamically turn on / off the .lazy modifier on v-model. (modelModifiers prop won't work with .lazy modifier.) This is why I had to duplicate the <input> and
<textarea> elements.

Fixes the issue that a user was able to input an invalid number to
`Numberinput`. The underlying `Input` uses `v-model` instead of the
combination of the `:value` binding and the `@input` and `@change` event
handlers to work around the upstream issue introduced by Vue 3.4.28.
(See the background) The `onInput` and `onChange` methods of `Input` no
longer update the model value but just run validation.

Due to the limitation of `v-model`, almost the same `<input>` and
`<textarea>` elements has to be repeated to implement the variants of
the `lazy` prop. (See the background)

Note that this commit will cause type errors on the `v-model` bindings
of `<textarea>`. The error will be fixed with Vue 3.4.6 or higher.

Background:

`Numberinput` should reject numbers which are not equal to the basis for
stepping, and the basis is derived from the `value` attribute unless the
`min` is given. Before Vue 3.4.28, updating the `value` prop did not
affect the `value` attribute, however, Vue 3.4.28 or higher introduced a
tweak that updates the `value` attribute when the `value` prop is
changed. Since the `value` attribute which defines the basis moves, the
validation of `Numberinput` can be messed up. The problematic tweak is
not applied to `v-model` which the underlying `Input` did not use.

The reason why `Input` did not use `v-model` was to deal with the `lazy`
prop. While `v-model` needs an optional `.lazy` modifier if the `lazy`
prop is `true`, unfortunately, we cannot dynamically turn on / off the
`.lazy` modifier on `v-model`. (`modelModifiers` prop won't work with
`.lazy` modifier`.) This is why I had to duplicate the `<input>` and
`<textarea>` elements.
More comprehensively tests the `lazy` prop of `Input`. Uses actual
"input" and "change" events through the underlying `<input>` instead of
`onInput` and `onChange` calls. Btw, `onInput` and `onChange` no longer
update the model value.
@kikuomax
Copy link
Collaborator Author

kikuomax commented Feb 3, 2025

The following type errors will be fixed if we upgrade Vue to 3.4.6 or higher:

Error: src/components/input/Input.vue(30,17): error TS2322: Type 'string | number | null | undefined' is not assignable to type 'string | number | readonly string[] | undefined'.
Error: src/components/input/Input.vue(61,17): error TS2322: Type 'string | number | null | undefined' is not assignable to type 'string | number | readonly string[] | undefined'.

@kikuomax kikuomax requested a review from wesdevpro February 3, 2025 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant