Skip to content

Is there any opportunities to share own methods in a custom directive around bind/unbind/update? #314

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

Closed
Jinjiang opened this issue Jun 14, 2014 · 13 comments

Comments

@Jinjiang
Copy link
Member

For example, I would add a event listener when bind and remove it when unbind. So I wrote:

Vue.directive('xxx', {
  bind: function () {
    this.vm.handler = function () {...};
    this.el.addEventListener('click', this.vm.handler);
  },
  unbind: function () {
    this.el.removeEventListener('click', this.vm.handler);
    delete this.vm.handler;
  }
});

But the handler is exposed out to vm.
Is there any better way?

Thanks.

@Jinjiang Jinjiang changed the title Is there any opportunities to share its own methods around bind/unbind/update? Is there any opportunities to share own methods in a custom directive around bind/unbind/update? Jun 14, 2014
@bpierre
Copy link

bpierre commented Jun 14, 2014

Why not emit an event on the VM?

Vue.directive('xxx', {
  bind: function () {
    this.handler = function () {
      this.vm.$emit('xxx:click', 'param1' /* , … */);
    }.bind(this);
    this.el.addEventListener('click', this.handler);
  },
  unbind: function () {
    this.el.removeEventListener('click', this.handler);
  }
});

@yyx990803
Copy link
Member

You can just attach the handler to this, which is the directive instance.

@Jinjiang
Copy link
Member Author

@bpierre I just want to set a private handler of the directive, not dispatched abroad. But the same as @yyx990803 's advice that your demo also uses this.
Thanks. That's really cool!

@jasonbodily
Copy link

Correct me if I'm wrong, but this is no longer a possibility in Vue 2.0 because there is no this object. How would one maintain reference to a function for registration and unregistration now? The documentation says:

Apart from el, you should treat these arguments as read-only and never modify them. If you need to share information across hooks, it is recommended to do so through element’s dataset.

which can't be done because the dataset property is a StringMap.

@nevcos
Copy link

nevcos commented Nov 27, 2017

@yyx990803: I'm facing the same issue as @jasonbodily.
On v2 which is the proper way to save the handler function reference to unbind it later?

@HcySunYang
Copy link
Member

@jasonbodily @nevcos Do not know if this can meet your need?

Vue.directive('xxx', {
  bind: function (el) {
    const handler = () => { ... }
    el.addEventListener('someEvent', handler)
    el.$destroy = () => el.removeEventListener('someEvent', handler)
  },
  unbind: function (el) {
    el.$destroy()
  }
});

Simply use the el parameter and define the destroy function in the bind hook

@nevcos
Copy link

nevcos commented Nov 28, 2017

@HcySunYang Yes, it meets the need. I've actually implemented that way 👍 I was just wondering if it's the best way.
Thanks!

@FrankFang
Copy link

@HcySunYang It works but looks counter-intuitive :(

@lazarljubenovic
Copy link

The solution proposed by @HcySunYang looks like some kind of hack. $destroy is misleading as it seems like it's something vue-related. It's in fact an arbitrary key, using el.i_am_a_banana would work as well.

@yyx990803 Can you confirm that this is the intended correct way of achieving this? Or maybe we should not remove event listeners at all? Does Vue's DOM management guarantee that there will be no memory leak issues?

Hanks10100 pushed a commit to Hanks10100/vue that referenced this issue May 6, 2018
kyleslight pushed a commit to sin-group/base-ui that referenced this issue Aug 11, 2018
Summary:
使用 dom dataset 来缓存

以及

vuejs/vue#314

Test Plan: as above

Reviewers: #web_reviewers, kuilin

Reviewed By: #web_reviewers, kuilin

Differential Revision: https://code.yangqianguan.com/D45553
@a-kriya
Copy link

a-kriya commented Aug 31, 2018

@yyx990803 Any recommendation or advice for cleaning up event listeners with Vue 2.x will be greatly appreciated.

@fnlctrl
Copy link
Member

fnlctrl commented Aug 31, 2018

Normally you don't have to clean up listeners manually. If an element is destroyed (removed from DOM and no longer referenced by javascript), its listeners are also removed (or precisely, no longer referenced by the element thus able to be garbage collected), this is guaranteed by DOM and is not related to vue at all.

In most cases if you're not passing references of elements and event listeners around (and accidentally create memory leaks by retaining the references), you don't need to worry about cleaning up event listeners at all.

@lazarljubenovic
Copy link

@fnlctrl My worry is not my own code, but the actual framework code. "In most cases", sure, but something like Vue is not your usual piece of code.

That is, does Vue guarantee the pre-conditions you mentioned which DOM requires in order to remove its listeners?

@fnlctrl
Copy link
Member

fnlctrl commented Aug 31, 2018

Yes. Otherwise it would be a memory-leak bug (which is rare: https://github.com/vuejs/vue/issues?utf8=%E2%9C%93&q=memory+leak+label%3Abug+) and would be fixed.

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

10 participants