-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fix(vModel): hyphenated v-model should use modifiers #4850
Conversation
…hyphenate string in props and camelize in emits
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a test too?
@@ -113,7 +113,8 @@ export function emit( | |||
const isModelListener = event.startsWith('update:') | |||
|
|||
// for v-model update:xxx events, apply modifiers on args | |||
const modelArg = isModelListener && event.slice(7) | |||
const eventArg = isModelListener && event.slice(7) | |||
const modelArg = eventArg && (hyphenate(eventArg) in props ? hyphenate(eventArg) : camelize(eventArg)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this also do it:
const modelArg = eventArg && (hyphenate(eventArg) in props ? hyphenate(eventArg) : camelize(eventArg)) | |
const modelArg = eventArg && (eventArg in props ? eventArg : hyphenate(eventArg)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, i have add a testcase, and i add hyphenate
because it may like this
this.$emit('update:firstName', ' one ')
// in parent
v-model:first-name.trim="firstName"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's been nearly two years since this was opened, but I think this PR is still relevant with the latest main
branch and I have a suggestion.
I believe there's a third option that needs to be handled here, not just camel and kebab case. I think an exact match also needs to be considered.
Here's an example:
While I accept it is not common practice, it is possible for the v-model
argument to be written in neither camel case nor kebab case. In the example above I use v-model:abc-DEF.trim
. This does work with the current code on main
, but it no longer works with the change proposed in this PR.
Even though it's an edge case, I think it's important that an exact match should always work. Case coercion logic shouldn't change that.
There's similar logic further down the same file for determining the value of handlerName
. That tries an exact match first, before considering any case changes. I think the logic for modifiers should be consistent with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've opened #9609, which builds on the work done here and also covers the case I mentioned above.
…don't match (vuejs#9609) close vuejs#4848 close vuejs#4850 (based on commits from vuejs#4850) Co-authored-by: zhaozhongyu <zhaozhongyu@xunlei.com> Co-authored-by: Evan You <evan@vuejs.org>
…hyphenate string in props and camelize in emits
fix: #4848