Skip to content

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

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

Open
KaelWD opened this issue Jun 7, 2019 · 5 comments
Open

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

KaelWD opened this issue Jun 7, 2019 · 5 comments

Comments

@KaelWD
Copy link
Contributor

KaelWD commented Jun 7, 2019

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
Copy link
Member

posva commented Jun 7, 2019

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

@KaelWD
Copy link
Contributor Author

KaelWD commented Jun 7, 2019

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
Copy link
Contributor Author

KaelWD commented Aug 6, 2019

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.

KaelWD added a commit to vuetifyjs/vuetify that referenced this issue Sep 1, 2019
johnleider added a commit to vuetifyjs/vuetify that referenced this issue Sep 23, 2019
* fix(perf): avoid useless re-render on parent update

fixes #6201
see vuejs/vue#7257 and vuejs/vue#10115

* test: disable sync

see vuejs/vue-test-utils#1130

* test: disable more sync, skip unfixable tests

* Update packages/vuetify/src/mixins/binds-attrs/index.ts

Co-Authored-By: John Leider <[email protected]>
@gaokun
Copy link

gaokun commented Sep 22, 2020

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
Copy link

gaokun commented Sep 22, 2020

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 />
  1. add different key to these two components
<my-button v-if="visible" key="a" />
<my-button v-else="visible" key="b" detail />
  1. 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
  },

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

No branches or pull requests

3 participants