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

v-bind="$props" injects inappropriate attributes into children #5606

Closed
dotBits opened this issue May 5, 2017 · 8 comments
Closed

v-bind="$props" injects inappropriate attributes into children #5606

dotBits opened this issue May 5, 2017 · 8 comments

Comments

@dotBits
Copy link

dotBits commented May 5, 2017

Version

2.3.2

Reproduction link

http://jsfiddle.net/cdev/g83wrcu9/4/

Steps to reproduce

  1. Define 2 components with a single prop on each one
  2. Use them in a Vue instance
  3. Pass props with v-bind="$props" to each one of them
  4. Inspect HTML

What is expected?

Only props defined in children should be used, the others should be ignored, at least not injected as HTML attributes because they turn markup as invalid

What is actually happening?

Props not defined in child component are injected as attributes and this invalidates HTML markup

@yyx990803
Copy link
Member

This is expected behavior. If you don't want this you need to either explicitly bind parent props or declare the props in the child.

@dotBits
Copy link
Author

dotBits commented May 5, 2017

@yyx990803 in the linked example props are declared in the child but the problem persists. Until 2.1.0 was working with v-bind.prop as documentation says https://vuejs.org/v2/api/#v-bind

@yyx990803 yyx990803 reopened this May 5, 2017
@dotBits
Copy link
Author

dotBits commented May 5, 2017

I got what do you mean with "...declare the props in the child", you say ALL props it might receive. But this would force to declare input props for a button for example (and vice versa), which is not a good pattern

@posva
Copy link
Member

posva commented May 5, 2017

@dotBits
I'd also say this is expected behaviour, these three things should be equivalent, and they currently are:

<Test v-bind="$props"></Test>
<Test v-bind="{ a, b }"></Test>
<Test :a="a" :b="b"></Test>

If you bind b but Test doesn't have a prop named b, it will pass it down, in this case, to the root element of Test

Also, I don't understand what you mean by it was workinng until 2.1.0 because $props was introduced in 2.2.0

@dotBits
Copy link
Author

dotBits commented May 5, 2017

@posva the problem is when you have 2 or more children components at the same level, they can have different props but they receive the whole set of props and what is not declared as prop in the child, will be an HTML attribute of that child element

// Form $props are { a: 'x', b: 'y', c: {foo: 'bar'} }
<Form>
    // Input has just { props: {a: String} } but HTML attributes are rendered with b="y" c="[Object object]"
    <Input v-bind="$props"></Input>
    // Button has just { props: {b: String, c: Object} } but HTML attributes are rendered with a="x"
    <Button v-bind="$props"></Button>
</Form>

We're implementing a components pattern library based on Atomic design system (Brad Frost), which means to include even small elements (like buttons and call to actions) in dozens of different context with more than 3 or 4 nested levels.
Explicitly declare those props is a huge overhead in our case, because we have a very granular distribution of Atoms, Molecules, Organisms and so on...

Yes you're right, the last working version with .prop modifier is 2.2.0-beta.1 I've created a fiddle which reproduces behavior with this version http://jsfiddle.net/cdev/uk2v3qfe/1/

@posva
Copy link
Member

posva commented May 5, 2017

Oh, I see, you seem to have misunderstood the prop modifier, there's more info in the docs. It can bind things as innerHTML and scrollTop. You don't need it in your case
You'll have to bind props explicitly. In your fiddle you're doing the same as

<Test :a="a" :b="b"></Test>

And that is going to produce wrong markup. Keep in mind ignoring other pops in v-bind would also be a breaking change

@posva posva closed this as completed May 5, 2017
@dotBits
Copy link
Author

dotBits commented May 5, 2017

@posva got it. I'm about to try with a custom directive to filter only owned props, any suggestion on how to approach it?

@posva
Copy link
Member

posva commented May 5, 2017

No, I suggest you to explicitly bind the props you need 😛
A component with a render function should be easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants