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

Attribute Fallthrough Behavior #27

Closed
yyx990803 opened this issue Mar 21, 2019 · 13 comments
Closed

Attribute Fallthrough Behavior #27

yyx990803 opened this issue Mar 21, 2019 · 13 comments

Comments

@yyx990803
Copy link
Member

yyx990803 commented Mar 21, 2019

Draft based on #5


  • Start Date: 2019-03-15
  • Target Major Version: 3.x
  • Reference Issues: N/A
  • Implementation PR: N/A

Summary

  • Disable implicit attribute fall-through to child component root element

  • Remove inheritAttrs option

Basic example

To replicate 2.x behavior in templates:

<div v-bind="$attrs">hi</div>

In render function:

import { h } from 'vue'

export default {
  render() {
    return h('div', this.$attrs, 'hi')
  }
}

Motivation

In 2.x, the current attribute fallthrough behavior is quite implicit:

  • class and style used on a child component are implicitly applied to the component's root element. It is also automatically merged with class and style bindings on that element in the child component template.

    • However, this behavior is not consistent in functional components because functional components may return multiple root nodes.

    • With 3.0 supporting fragments and therefore multiple root nodes for all components, this becomes even more problematic. The implicit behavior can suddenly fail when the child component changes from single-root to multi-root.

  • attributes passed to a component that are not declared by the component as props are also implicitly applied to the component root element.

    • Again, in functional components this needs explicit application, and would be inconsistent for 3.0 components with multiple root nodes.

    • this.$attrs only contains attributes, but excludes class and style; v-on listeners are contained in a separate this.$listeners object. There is also the .native modifier. The combination of inheritAttrs, .native, $attrs and $listeners makes props passing in higher-order components unnecessarily complex. The new behavior makes it much more straightforward: spreading $attrs means "pass everything that I don't care about down to this element/component".

    • class and style are always automatically merged, and are not affected by inheritAttrs.

The fallthrough behavior has already been inconsistent between stateful components and functional components in 2.x. With the introduction of fragments (the ability for a component to have multiple root nodes) in 3.0, the fallthrough behavior becomes even more unreliable for component consumers. The implicit behavior is convenient in cases where it works, but can be confusing in cases where it doesn't.

In 3.0, we are planning to make attribute fallthrough an explicit decision of component authors. Whether a component accepts additional attributes becomes part of the component's API contract. We believe overall this should result in a simpler, more explicit and more consistent API.

Detailed design

  • inheritAttrs option will be removed.

  • .native modifier will be removed.

  • Non-prop attributes no longer automatically fallthrough to the root element of the child component (including class and style). This is the same for both stateful and functional components.

    This means that with the following usage:

    const Child = {
      props: ['foo'],
      template: `<div>{{ foo }}</div>`
    }
    
    const Parent = {
      components: { Child },
      template: `<child foo="1" bar="2" class="bar"/>`
    }

    Both bar="2" AND class="bar" on <child> will be ignored.

  • this.$attrs now contains everything passed to the component except those that are declared as props or custom events. This includes class, style, v-on listeners (as onXXX properties). The object will be flat (no nesting) - this is possible thanks to the new flat VNode data structure (discussed in Render Function API Change).

    To explicitly inherit additional attributes passed by the parent, the child component should apply it with v-bind:

    const Child = {
      props: ['foo'],
      template: `<div v-bind="$attrs">{{ foo }}</div>`
    }

    This also applies when the child component needs to apply $attrs to a non-root element, or has multiple root nodes:

    const ChildWithNestedRoot = {
      props: ['foo'],
      template: `
        <label>
          {{ foo }}
          <input v-bind="$attrs">
        </label>
      `
    }
    
    const ChildWithMultipleRoot = {
      props: ['foo'],
      template: `
        <label :for="$attrs.id">{{ foo }}</label>
        <input v-bind="$attrs">
      `
    }

    In render functions, if simple overwrite is acceptable, $attrs can be merged using object spread. But in most cases, special handling is required (e.g. for class, style and onXXX listeners). Therefore a cloneVNode helper will be provided. It handles the proper merging of VNode data:

    import { h, cloneVNode } from 'vue'
    
    const Child = {
      render() {
        const inner = h(InnerComponent, {
          foo: 'bar'
        })
        return cloneVNode(inner, this.$attrs)
      }
    }

    The 2nd argument to cloneVNode is optional. It means "clone the vnode and add these additional props". The cloneVNode helper serves two purposes:

    • Avoids mutating the original VNode
    • Handles special merging logic for class, style and event listeners

    Inside render functions, the user also has the full flexibility to pluck / omit any props from $attrs using 3rd party helpers, e.g. lodash.

Removing Unwanted Listeners

With flat VNode data and the removal of .native modifier, all listeners are passed down to the child component as onXXX functions:

<foo @click="foo" @custom="bar" />

compiles to:

h(foo, {
  onClick: foo,
  onCustom: bar
})

When spreading $attrs with v-bind, all parent listeners are applied to the target element as native DOM listeners. The problem is that these same listeners can also be triggered by custom events - in the above example, both a native click event and a custom one emitted by this.$emit('click') in the child will trigger the parent's foo handler. This may lead to unwanted behavior.

Props do not suffer from this problem because declared props are removed from $attrs. Therefore we should have a similar way to "declare" emitted events from a component. There is currently an open RFC for it by @niko278.

Event listeners for explicitly declared events will be removed from $attrs and can only be triggered by custom events emitted by the component via this.$emit.

Drawbacks

  • Fallthrough behavior is now disabled by default and is controlled by the component author. If the component is intentionally "closed" there's no way for the consumer to change that. This may cause some inconvenience for users accustomed to the old behavior, especially when using class and style for styling purposes, but it is the more "correct" behavior when it comes to component responsibilities and boundaries. Styling use cases can be easily worked around with by wrapping the component in a wrapper element. In fact, this should be the best practice in 3.0 because the child component may or may not have multiple root nodes.

  • For accessibility reasons, it should be a best practice for components that are shipped as libraries to always spread $attrs so that any aria-x attributes can fallthrough. However this is a straightforward / mechanical code change, and is more of an educational issue. We could make it common knowledge by emphasizing this in all our information channels.

Alternatives

N/A

Adoption strategy

Documentation

This RFC discusses the problem by starting with the 2.x implementation details with a lot of history baggage so it can seem a bit complex. However if we were to document the behavior for a new user, the concept is much simpler in comparison:

  • For a component without explicit props and events declarations, everything passed to it from the parent ends up in $attrs.

  • If a component declares explicit props, they are removed from $attrs.

  • If a component declares explicit events, corresponding onXXX listeners are removed from $attrs.

  • $attrs essentially means extraneous attributes,, or "any attributes passed to the component that hasn't been explicitly handled by the component".

Migration

This will be one of the changes that will have a bigger impact on existing code and would likely require manual migration.

  • We will provide a warning when a component has unused extraneous attributes (i.e. non-empty $attrs that is never used during render).

  • For application code that adds class / style to child components for styling purposes: the child component should be wrapped with a wrapper element.

  • For higher-order components or reusable components that allow the consumer to apply arbitrary attributes / listeners to an inner element (e.g. custom form components that wrap <input>):

    • Declare props and events that are consumed by the HOC itself (thus removing them from $attrs)

    • Refactor the component and explicitly add v-bind="$attrs" to the target inner component or element. For render functions, apply $attrs with the cloneVNode helper.

    • If a component is already using inheritAttrs: false, the migration should be relatively straightforward.

We will need more dogfooding (migrating actual apps to 3.0) to provide more detailed migration guidance for this one, since the migration cost heavily depends on usage.

@Akryum
Copy link
Member

Akryum commented Mar 21, 2019

  • It's unclear to me what cloneVNodedoes exactly :(
  • We should definitely be able to declare events just like we do with props and remove them from $attrs, otherwise this may be very confusing. Also typically you want to customize some events behavior in a HOC and re-emit them as custom events, so some it would be even more useful.

@yyx990803
Copy link
Member Author

@Akryum

It's unclear to me what cloneVNode does exactly :(

It clones the VNode :) The 2nd argument to cloneVNode is optional. It means "clone and add these additional props". It serves a few purposes:

  • Avoid mutating the original VNode
  • Handles special merging logic for class, style and event listeners

We should definitely be able to declare events just like we do with props and remove them from $attrs

What about a new option called emits? (to differentiate it from the old events option)

export default {
  props: ['foo'],
  emits: ['click']
}

I believe we've had similar feature requests in the past, but we did not implement it because we felt it didn't provide enough value. Now this would serve a few useful purposes:

  • Declared events are removed from this.$attrs to avoid conflict
  • Documents what the component emits - maybe can be used by Vetur for autocompletion? /cc @octref
  • Enable runtime validation of required event listeners / payload type check

@nekosaur
Copy link
Contributor

What about a new option called emits? (to differentiate it from the old events option)

Being able to declare slots in the same way could similarly help with automatic doc generation and autocompletion. But on its own perhaps that's not enough added value to be considered?

@LinusBorg
Copy link
Member

I believe we've had similar feature requests in the past, but we did not implement it because we felt it didn't provide enough value.

There's even a current RFC proposal for such an option

@LinusBorg
Copy link
Member

LinusBorg commented Mar 22, 2019

There's also the topic of custom directives used on components. those also don't work anymore (or atleast don't make sense anymore), since we now have Fragements support and there's no guaranteed root element.

Were should we document this? Here? or in #29 ?

@KaelWD
Copy link
Contributor

KaelWD commented Mar 22, 2019

Aren't directives going to take VNodes now instead of an element?

@LinusBorg
Copy link
Member

If so I missed the memo ... :-P

@KaelWD
Copy link
Contributor

KaelWD commented Mar 22, 2019

#2

Now are internally on-vnode hooks with the exact same lifecycle as components

Maybe I misinterpreted that

@posva
Copy link
Member

posva commented Mar 23, 2019

The implicit behavior is convenient in cases where it works, but can be confusing in cases where it doesn't

Whereas I agree with that, isn't automatic inherit still the most common case (except for multiple children). When developing components in apps, I won't add the v-bind="$attrs" until I have the need to add a custom class or attribute, so I will have to check if it's already there. Whereas I almost never find myself in the opposite situation: the component picking up something I didn't want to.

How are on* events going to work exactly? Are we going to bind to the on* dom attributes directly? Isn't this a deprecated practice in favor of addEventListener?

@yyx990803
Copy link
Member Author

yyx990803 commented Mar 23, 2019

@LinusBorg @KaelWD

There's also the topic of custom directives used on components.

That's a good point. Yeah I don't think it makes sense anymore.

The directive API is indeed going to change so that will be another RFC.


@posva

Whereas I agree with that, isn't automatic inherit still the most common case

I think explicit is better than implicit here. It's simply impossible to keep it consistent with the implicit behavior. When fragments become available, it is very inconsistent if the user have to add v-bind="$attrs" based on whether the template has one or multiple root nodes.

How are on* events going to work exactly?

v-on:click is compiled to onClick and then handled by the renderer as addEventListener('click', ...). In addition, when a component calls this.$emit('foo') will fire the onFoo handler on the component vnode if there is one.

@posva
Copy link
Member

posva commented Mar 25, 2019

I also prefer explicit. I think the implicit behaviour here is something a lot of users like, so I want to be able to explain correctly.

v-on:click is compiled to onClick

I meant that if $attrs contains onListeners, if you do <div v-bind="$attrs"/>, we are binding a dom property onClick (which doesn't exist) to the div. Or is v-bind going to handle onAttribute properties automatically as events? (Which would be a breaking change)

@yyx990803
Copy link
Member Author

@posva

if $attrs contains onListeners, if you do

, we are binding a dom property onClick (which doesn't exist) to the div

That's what I mentioned in the "Unresolved questions".

is v-bind going to handle onAttribute properties automatically as events?

Yes. This is not really a breaking change since attributes and props need to be nested in 2.x. You can still declare props named onXXX and they will be removed from $attrs.

@yyx990803
Copy link
Member Author

Published: vuejs/rfcs#26

@github-actions github-actions bot locked and limited conversation to collaborators Nov 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants