vuejs / vue

This is the repo for Vue 2. For Vue 3, go to https://github.com/vuejs/core
http://v2.vuejs.org
MIT License
207.84k stars 33.68k forks source link

Unnecessary renders on parent update when $attrs is bound #10115

Open KaelWD opened 5 years ago

KaelWD commented 5 years ago

Version

2.6.10

Reproduction link

https://codepen.io/anon/pen/zQVRgG?editors=1010

Steps to reproduce

Type something into the first field

Uncomment line 8 or 14 then try again

What is expected?

In console:

Render a

What is actually happening?

In console:

Render a
Render b
posva commented 5 years ago

This is basically the same problem as with $listeners at https://github.com/vuejs/vue/issues/7257 It's because we read parent.attrs: https://github.com/vuejs/vue/blob/dev/src/core/instance/lifecycle.js#L260

KaelWD commented 5 years ago

Ah yep, missed that one.

Looks like we have c('field', { attrs: { "title": "b" } }), then title is removed at extractProps leaving an empty object, so the || emptyObject doesn't apply.
I'll leave this open as the fix will probably be different.

KaelWD commented 5 years ago

The workaround I'm using for now is to mutate a single object instead:

data: () => ({
  $_attrs: {},
  $_listeners: {},
}),
watch: {
  // Work around unwanted re-renders: https://github.com/vuejs/vue/issues/10115
  // Make sure to use `v-bind="$data.$_attrs"` instead of `v-bind="$attrs"`
  $attrs: {
    handler (val) {
      for (const attr in val) {
        this.$set(this.$data.$_attrs, attr, val[attr])
      }
    },
    immediate: true
  },
  $listeners: {
    handler (val) {
      for (const listener in val) {
        this.$set(this.$data.$_listeners, listener, val[listener])
      }
    },
    immediate: true
  }
},

https://codepen.io/anon/pen/rXpGbj?editors=1010

I 100% guarantee there's bugs in this btw.

gaokun commented 4 years ago

The workaround I'm using for now is to mutate a single object instead:

data: () => ({
  $_attrs: {},
  $_listeners: {},
}),
watch: {
  // Work around unwanted re-renders: https://github.com/vuejs/vue/issues/10115
  // Make sure to use `v-bind="$data.$_attrs"` instead of `v-bind="$attrs"`
  $attrs: {
    handler (val) {
      for (const attr in val) {
        this.$set(this.$data.$_attrs, attr, val[attr])
      }
    },
    immediate: true
  },
  $listeners: {
    handler (val) {
      for (const listener in val) {
        this.$set(this.$data.$_listeners, listener, val[listener])
      }
    },
    immediate: true
  }
},

https://codepen.io/anon/pen/rXpGbj?editors=1010

I 100% guarantee there's bugs in this btw.

Yes, need to delete attrs which wasn't in $attrs any more.

gaokun commented 4 years ago

Consider this scenario:

<my-button v-if="visible" />
<my-button v-else detail />

If we toggle visible once, detail will be in $_attrs even visible is true. I think the root cause is vue v-dom diff policy.

There are three ways to help this out:

  1. use v-show instead of v-if
    <my-button v-show="visible" />
    <my-button v-show="!visible" detail />
  2. add different key to these two components
    <my-button v-if="visible" key="a" />
    <my-button v-else="visible" key="b" detail />
  3. delete attributes which wasn't in $attrs any more. (like 'detail') [the same with $listeners]

    $attrs: {
    handler (val) {
      const oldKeys = Object.keys(this.$data.$_attrs)
      for (const attr in val) {
        this.$set(this.$data.$_attrs, attr, val[attr])
        const index = oldKeys.indexOf(attr)
        if (index > -1) {
          oldKeys.splice(index, 1)
        }
      }
    
      for (const attr of oldKeys) {
        this.$delete(this.$data.$_attrs, attr);
      }
    },
    immediate: true
    },