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

Improve behaviour of the "on" lint #264

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

gitnik
Copy link

@gitnik gitnik commented Nov 13, 2015

  • The lint will from now on only handle "$on" calls, as "$watches" are automatically removed upon destroying the scope. I therefore also changed the name of the lint to match what is does.
  • The lint will no longer complain about not unsubscribing from the $rootScope. While there can be cases where this is valid, most of the time it isn't
  • Adapt tests
  • Adapt documentation

* The lint will from now on only handle "$on" calls, as "$watches" are automatically removed upon destroying the scope.
* The lint will no longer complain about not unsubscribing from the $rootScope. While there can be cases where this is valid, most of the time it isn't
* Adapt tests
* Adapt documentation
@gitnik gitnik mentioned this pull request Nov 13, 2015
@gitnik
Copy link
Author

gitnik commented Nov 13, 2015

Fixes #231

@EmmanuelDemey
Copy link
Owner

Can you do your PR on the development branch? Thanks by the way ;)

@charneykaye
Copy link

Any reason not to do this?

$rootScope.$on('$destroy', $rootScope.$on(EVENTS.changeRegion, function() {
  vm.nowRegion = OpsService.getRegion();
}));

@mustela
Copy link

mustela commented Jan 20, 2016

@gitnik any plan to work on this one?

Thanks

@briananderson1222
Copy link

Would love to see this

@ivan-zakharchuk
Copy link

@EmmanuelDemey EmmanuelDemey changed the base branch from master to development December 24, 2016 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants