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

Component v-model API change #31

Merged
merged 7 commits into from Nov 12, 2019
Merged

Component v-model API change #31

merged 7 commits into from Nov 12, 2019

Conversation

@yyx990803
Copy link
Member

yyx990803 commented Apr 10, 2019

Adjust v-model API when used on custom components.

This builds on top of #8 (Replace v-bind's .sync with a v-model argument).

Rendered

@CyberAP

This comment has been minimized.

Copy link

CyberAP commented Apr 12, 2019

Having an additional attribute for custom elements to support native-like behaviour for v-model is kind of strange. I would love to see v-model support an extra argument for that case with a special symbol, for example:

<custom-input v-model:enabled@checkbox="inputEnabled" />
@posva

This comment has been minimized.

Copy link
Member

posva commented Apr 12, 2019

@CyberAP I also thought about that, but a modifier woud make more sense instead of @: v-model:enabled.checkbox="inputEnabled"

@leopiccionia

This comment has been minimized.

Copy link

leopiccionia commented Apr 12, 2019

It would be nice if lazy was a type, whichever syntax is ultimately chosen.

@GerardRodes

This comment has been minimized.

Copy link

GerardRodes commented Jun 22, 2019

Why

v-model:name="value"

Instead of

:name.sync="value"
@edcoreweb

This comment has been minimized.

Copy link

edcoreweb commented Jun 24, 2019

This will be breaking a lot of packages which implement v-model on custom components.

In 3.0, the model option will be removed. v-model="foo" (without argument) on a component compiles to the following instead:

Why not leave the model option for the first few 3.x versions, deprecate it so that the upgrade path will be easier for existing Vue 2.x packages ?

@beeplin

This comment has been minimized.

Copy link

beeplin commented Jun 24, 2019

@edcoreweb i think such option removals must happen in major version bumps like 2->3 or 3->4. If you don't remove it in 3.0.0, you will not be able to remove it until 4.0.0.

@backbone87

This comment has been minimized.

Copy link

backbone87 commented Jul 17, 2019

wouldnt it be better to name the default model value prop default? this aligns with slots and es modules.
maybe the BC break is a little harder since default is more likely to conflict than modelValue, but not that likely and there are BC breaks regarding this anyway, so might as well take the pill now for improved consistency.

@atilkan

This comment has been minimized.

Copy link

atilkan commented Oct 31, 2019

This is much better.

@yyx990803

This comment has been minimized.

Copy link
Member Author

yyx990803 commented Nov 6, 2019

This RFC is now in final comments stage. An RFC in final comments stage means that:

The core team has reviewed the feedback and reached consensus about the general direction of the RFC and believe that this RFC is a worthwhile addition to the framework.
Final comments stage does not mean the RFC's design details are final - we may still tweak the details as we implement it and discover new technical insights or constraints. It may even be further adjusted based on user feedback after it lands in an alpha/beta release.
If no major objections with solid supporting arguments have been presented after a week, the RFC will be merged and become an active RFC.

Co-Authored-By: Jacek Karczmarczyk <jkarczm@gmail.com>
@sqal

This comment has been minimized.

Copy link

sqal commented Nov 6, 2019

Regarding modelModifiers prop. I noticed that if I use more than one v-model on the component with different modifiers, then compiler creates modelModifiers prop with modifiers only from the first v-model. is this a bug?

Demo: https://vue-next-template-explorer.netlify.com/#%7B%22src%22%3A%22%3CComp%20%20%5Cr%5Cn%20%20v-model%3Aname.a.b%3D%5C%22name%5C%22%5Cr%5Cn%20%20v-model%3Aemail.c%3D%5C%22email%5C%22%5Cr%5Cn%2F%3E%22%2C%22options%22%3A%7B%22mode%22%3A%22module%22%2C%22prefixIdentifiers%22%3Afalse%2C%22hoistStatic%22%3Afalse%2C%22cacheHandlers%22%3Afalse%7D%7D

I believe the model is `text`, not `foo`, in `v-model.foo.bar="text"`
@yyx990803

This comment has been minimized.

Copy link
Member Author

yyx990803 commented Nov 7, 2019

@sqal yeah, good catch! We need to generate different prop for v-model with args.

Update: fixed in vuejs/vue-next@f178874

@yyx990803 yyx990803 changed the title v-model API change Component v-model API change Nov 7, 2019
@CyberAP

This comment has been minimized.

Copy link

CyberAP commented Nov 7, 2019

Why is it not possible to write model handler in render function with modifiers?
Example: onUpdate:foo.number.

Or at least define it as an object?

h('Foo', {
  'onUpdate:bar': {
    handler: val => (baz = val),
    trim: true,
  }
})
@denisinvader

This comment has been minimized.

Copy link

denisinvader commented Nov 7, 2019

I think, writing 'onUpdate:modelValue' in JSX is quite ugly. So, to implement an input in a shared library that fits templates and render functions ways you have to support two props and two events (for single-prop v-model) or write uncommon (less native/classic) JSX

@yyx990803

This comment has been minimized.

Copy link
Member Author

yyx990803 commented Nov 8, 2019

@CyberAP @denisinvader the format is optimized for the consuming component and the compiler. For manual render functions you can always build a little helper to make the usage read nicer:

withModel(h(SomeComp), {
  prop: 'foo', // optional
  value: foo,
  update: value => (foo = value),
  modifiers: { trim: true }
})

@denisinvader Vue will have its own JSX transform which gives us the ability to handle the transform for you. The 2.x transform only handles <Comp vModel={foo} />, we do need to discuss how the syntax should look like for v-model with arguments and modifiers in JSX for 3.0.

@yyx990803 yyx990803 merged commit 83d8780 into master Nov 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.