Attribute fallthrough behavior

[yyx990803]

Current proposal:

• Remove inhertiAttrs option
• Remove nativeOn
• Remove automatic attribute fallthrough in all cases
• The only, consistent way to enable attribute fallthrough is to explicitly spread $attrs on desired target element
  • In template: <div v-bind="$attrs">
  • In render functions: return cloneVNode(h('div'), this.$attrs)
  • note if component has no declared props, $attrs will be empty and everything will be in $props, and it should just spread / pass on $props instead. $attrs will just point to $props which includes everything passed to the component.

Rationale

• The fallthrough behavior has already been inconsistent between stateful components and functional components in 2.x. With the introduction of fragments, 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. By making this an explicit decision of component authors, whether a component accepts additional attributes becomes part of the component's API contract, overall resulting in more consistency.

• The combination of inheritAttrs, nativeOn, $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".

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, but can be easily worked around with by wrapping the component in a wrapper element.

• For accessibility reasons, it should be a best practice for components that are shipped as libraries to always spread $attrs. 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.

Side Benefit

Not spreading $attrs actually improves performance.

[chrisvfritz]

Great overview!

Or maybe $attrs should just point to $props in this case?

I think that makes sense. Then users never have to refactor v-bind="$props" to v-bind="$attrs" when they start defining props.

[johnleider]

I like all of these changes. I felt nativeOn made things confusing for users. The fall-through will make it easier to construct components.

[chrisvfritz]

cloneVNode(h('div'), this.$attrs)

@yyx990803 Quick question: why do we need cloneVNode here instead of just doing h('div', this.$attrs)?

[yyx990803]

Oh, that's only needed if you have your own data that you want to merge into.

[blake-newman]

I have some queries for this:

1. The $attrs does include class / style for merging? If not this may void my further questions.

$attrs will just point to $props which includes everything passed to the component.

Here is an example component in React:

import * as classNames from 'classnames'
import * as React from 'react'

interface ExampleProps extends React.HTMLProps<HTMLDivElement> {
  test?: boolean
}

export const Example: React.StatelessComponent<ExampleProps> = props => {
  const { className } = props

  const exampleClass = classNames({
    example: true,
    [className || '']: true,
  })

  return <div {...props} className={exampleClass} />
}

In Vue 2.x a similar component may be (typing retracted):

export const Example = ({ data }) => {
  return <div {...data}  class="example" />
}

For React:

• If you do not pass any props all attributes are transferred
  • <Example aria-test="test" data-test="test" tabIndex={0} className="test" />
  • <div aria-test="test" data-test="test" tabIndex="0" class="example test" ></div>
• If you do pass any props no attributes are transferred
  • <Example aria-test="test" data-test="test" tabIndex={0} className="test" test={true} />
  • <div class="example " ></div>
• If you explicitly pass attributes down like <div className={exampleClass} data-test={props['data-test']} then it is tranfered

All of the cases for React, in my opinion, hinder A11Y as you can not practically just transfer global attributes to an element.

For Vue:

All of the attributes are transferred, and class names styles are merged which is great for accessibility.

However there is one issue vuejs/vue#7557 which means everything is transferred, which is not ideal as every prop is directly transferred, and normally there is the mapping of props to attributes. The solution would be explicit like React or manually merge classes etc along with non global attributes stripped.

At attest we have a little snippet that strips all non global attributes from the attrs object, which means that all global attributes such as aria-* are passed through.

// https://github.com/vuejs/vue/issues/7557
function proxyRenderWithAttrsFix(component: any) {
  if (component.functional) {
    const _render = component.render
    component.render = (h: CreateElement, refs: RenderContext) => {
      if (!refs.data.attrs) {
        return _render(h, refs)
      }

      refs.data.attrs = stripNonGlobalAttrs(refs.data.attrs)
      return _render(h, refs)
    }
  }
  return component
}


const GLOBAL_NON_MERGABLE_ATTRS = /^((data-|aria-)|(accesskey|autocapitalize|contenteditable|contextmenu|dir|draggable|dropzone|hidden|is|item(id|prop|ref|scope|type)|lang|slot|spellcheck|tabindex|title|translate)$)/
function stripNonGlobalAttrs(attrs: { [key: string]: any }) {
  const _attrs = attrs || {}
  const newAttrs: { [K: string]: any } = {}
  const attrKeys = Object.keys(_attrs)
  const attrKeysLength = attrKeys.length
  for (let i = 0; i < attrKeysLength; i++) {
    const key = attrKeys[i]
    if (GLOBAL_NON_MERGABLE_ATTRS.test(key)) {
      newAttrs[key] = _attrs[key]
    }
  }

  return newAttrs
}

What this means is that we need to be careful with similarly named props as these will transfer, however, this does mean we get class/style merging.

This would be awesome if we can come to a solution that improves upon Reacts issues with functional components, so that we can have easy merging of class names and attribute tranfer. Any suggestions?

[yyx990803]

@blake-newman not entirely sure if I understand your point... but is the concern about spreading $attrs would overwrite existing class and style? In response to that:

• $attrs does contain class and style

• When you do the following, class and style are automatically merged (same for v-bind):

  cloneVNode(h('div', { class: 'foo' }), { class: 'bar' }) // -> class: 'foo bar'

[blake-newman]

No that is fine, the problem is when you have functional components and no defined props. This is common when using this definition: https://github.com/nickmessing/babel-plugin-jsx-vue-functional

Attributes will all flood to the dom, and with more complex functional component this is an issue.

export const Example = data => <div {...data} data-attr={data.props.value} />

will render every attribute with mergeds styles and class names <div value="500" data-attr="500">

The merging is great as it is by far less hindering to passing down aria roles - where is React you would need to explicitly pass through individual values to the element for those attributes to render

So with the Vue way, we will render everything out, but in React this does not happen. Both have their benefits and the above code snippet I shared partially resolves this issue by allowing only global attributes to be passed through to the element.

[yyx990803]

Right, but I think in those cases you are probably better off declaring the props explicitly.

[blake-newman]

In that case I think we should mimic React keeping class name and style merging but not passing through attributes on spread to element nodes. This means that you get best of both worlds.

Easy merging of classes and styles and explicit passing of props.

[yyx990803]

@blake-newman the problem with implicit class and style merging is that it does not always work - e.g. when the component returns a fragment. A simple refactoring can accidentally cause things to break.

[blake-newman]

Yes i agree, that is why you must still explicitly ask for $attrs to fall through for style and class merging.

This is what I propose which follows React following 2.x API as i'm unsure on the structure for 3.x:

This should be consistent for component fallthrough and element fallthrough

Case 1

JS:

(h, { data }) => cloneVNode(h('div', { class: { 'foo' } }), data)

JSX:

({ data }) => <div class="foo" {...data}>

Use:

<Example data-test="test" class="bar" />

Renders:

<div data-test="test" class="foo bar"></div>

Outcome: Attribute merging with full fallthrough if no attributes/props specified

Case 2

JS:

(h, { data }) => cloneVNode(h('div', { class: { 'foo' }, attrs: { 'data-test': data.props['data-test'] } }), data)

JSX:

({ data, props, attrs  }) => <div class="foo" data-test={props.dataTest || attrs['data-test']} {...data}>

Use:

<Example data-test="test" myProp="test" class="bar" />

Render:

<div class="foo bar" data-test="test"></div>

Outcome: Attribute merging, no fallthrough of additional props/attribute, must be explicit
Note: attrs object is unchanged but props is transformed to camelCase

This means you need to be more explicit but this prevents cases where props are rendered as attributes which is a big issue for our application. In hindsight our global attribute fallthrough is incorrect and i much prefer this behaviour as React does

[yyx990803]

@blake-newman I'm not sure if I can understand your proposal.

A few notes regarding v3:

Because VNode data is now flat, there's no longer the difference between data, props and attrs when you look at a VNode.

• On a plain element: class and style has special treatments. Fields that start with on are treated as handlers, and fields that contain uppercase letters are treated as DOM props. The rest are treated as attributes.

• On a component:

  • If the component has no declared props: entire data object is passed verbatim as $props. $attrs points to $props in this case.
  • If the component has declared props: the declared ones are extracted into $props, everything else goes into $attrs.

So Case 1 would look like this (note v3 also drops the leading h):

const Foo = props => cloneVNode(h('div', { class: 'foo' }), props)

If you want to use some of the props and only spread the rest:

const Foo =({ foo, ...attrs }) => cloneVNode(h('div', { class: 'foo', id: foo }), attrs)

Your Case 2 seems very convoluted and I can't really understand what you are trying to do. Maybe you should take a step back and explain "cases where props are rendered as attributes". But in v3, if the component has declared props, you'd get a clear separation between $props and $attrs and you are in full control of what to render and what not to render on the root element.

[blake-newman]

Okay, this makes a lot more sense, sorry for the confusion. My understanding was based on a 2.x issue where their vNode data was not flat, which would mean all information even if extracted would be transferred as attributes if you did not empty data.attrs object as this information exists on both props and attrs.

Now that it is flat, properties won't be transferred when extracting which will mitigate this issue.

[yyx990803]

Closed in favor of #27