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

input type=number doesn't work as intended #3813

Closed
jods4 opened this issue May 21, 2021 · 7 comments
Closed

input type=number doesn't work as intended #3813

jods4 opened this issue May 21, 2021 · 7 comments
Labels
need guidance The approach/solution in the PR is unclear and requires guidance from maintainer to proceed further.

Comments

@jods4
Copy link
Contributor

jods4 commented May 21, 2021

Version

3.0.11

Reproduction link

https://jsfiddle.net/4ou5ghLd/18/

Steps to reproduce

Vue tries to automatically apply a number modifier to v-model that are set on input[type=number], as can be seen in the code here:
https://github.com/vuejs/vue-next/blob/master/packages/runtime-dom/src/directives/vModel.ts#L52

Go to the repro link, try to enter a number in the textbox, you'll see the model contains a string, not a number

What is expected?

v-model set the ref to a number when used on input[type=number]

What is actually happening?

v-model sets the ref to a string, even if the number is valid


I put a breakpoint inside Vue and it seems like the source line linked above ran before other input properties are set.
The className is still empty and the type is still text (the default input type).

I recall a change was made on when v-model is applied to input vs other attributes because of an issue related to min and max, this could be a regression introduced by that change.

@jacekkarczmarczyk
Copy link
Contributor

Where is this behaviour described in docs? I can't find anything about it, neither in the guide/api reference nor in the migration guide.

@edison1105
Copy link
Member

Where is this behavior described in docs?

There is no such thing, Vue2 also does not have this behavior. .number is recommended.

Maybe the PR I submitted is not needed because of the increased runtime overhead.

@jacekkarczmarczyk
Copy link
Contributor

Then maybe || el.type === 'number' could just be removed?

@jods4
Copy link
Contributor Author

jods4 commented May 22, 2021

@jacekkarczmarczyk I found out about this feature by reading the code. I don't think I ever saw it mentioned in docs.

I'd say it's rather logical and nice to have, when you bind a type=number you kind of expect its value to be a number, don't you?
A bit like when you bind a type=checkbox you get a boolean.

A quick blame shows that the code was introduced by @yyx990803 in 3.0.0-alpha.0. The commit comment doesn't provide a lot of background:

feat(v-model): number/trim modifier + array checkbox support

Up to you what you want to do about it.
If it's not intended to work that way you prob. should remove the code.

Although I said above that it's very convenient to automatically get a number out of type=number, it's a bit unique in that you don't get a Date out of type=date for example. That would be easy enough by reading valueAsDate but would certainly be a significant breaking change.

@HcySunYang
Copy link
Member

Since this is an undocumented feature, I will not mark it as a bug, and @edison1105's PR can fix it, let’s wait for @yyx990803 ’s input.

@HcySunYang HcySunYang added the need guidance The approach/solution in the PR is unclear and requires guidance from maintainer to proceed further. label May 23, 2021
@posva
Copy link
Member

posva commented May 27, 2021

It would be problematic to have the modifier applied implicitly to v model as there would be no way to turn it off and the string version is more flexible

@hedefalk
Copy link

hedefalk commented Oct 13, 2021

@posva, @yyx990803

A general follow-up question. Can I somehow detect these things build-time? I got bitten of this since this typechecks and emits fine but the string is a number runtime:

<script setup lang="ts">

import { computed } from '@vue/reactivity';

const props = defineProps<{ modelValue: string }>()
const emit = defineEmits(['update:modelValue']);

const value = computed<string>({
    get() {
        return props.modelValue
    },
    set(val) {
        emit('update:modelValue', val);
    }
})
</script>

<template>
    <input type="number" v-model="value" />
</template>

But I guess that's the general case, not just for this string/number thing…?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
need guidance The approach/solution in the PR is unclear and requires guidance from maintainer to proceed further.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants