Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Debounced ng-model-options in v1.6.* fires $digests on each keypress (worked fine in v1.5.*) #16057

Closed
1 of 3 tasks
Nickproger opened this issue Jun 15, 2017 · 7 comments
Closed
1 of 3 tasks

Debounced ng-model-options in v1.6.* fires $digests on each keypress (worked fine in v1.5.*) #16057

Nickproger opened this issue Jun 15, 2017 · 7 comments
Assignees
@jbedard
Labels
component: forms
Milestone
1.6.x

Comments

@Nickproger
Copy link

I'm submitting a ...

  • bug report
  • feature request
  • other (Please do not submit support requests here (see above))

Current behavior:

Video here: https://www.screencast.com/t/TFhK7wyaG !!!
Angular 1.6.* - with ng-model-options="{debounce:1000}" fires $digest cycles on each keypress.
Angular 1.5.* - works perfect!
https://codepen.io/anon/pen/BZQQMG?editors=1010
https://codepen.io/anon/pen/EXNNqN?editors=1010

Expected / new behavior:

ng-model-options should work in Angular 1.6.* as it worked in Angular 1.5.*.

Minimal reproduction of the problem with instructions:

Angular version: 1.6.* has bug (worked fine in 1.5.*)

Browser: [all]

Anything else:
@valenburm
Copy link

+1

2 similar comments
@masterOL
Copy link

+1

@vpalamarchuk
Copy link

+1

@Narretz
Copy link
Contributor

Narretz commented Jun 16, 2017

Guys, stop commenting with +1 - this is what the reactions are for.

@Narretz Narretz added this to the 1.6.x milestone Jun 16, 2017
@jbedard
Copy link
Collaborator

jbedard commented Jun 17, 2017 •
edited
Loading

I think this was caused by c9dffde#diff-7e6919ccdaa77645580e865e271080eeR89. That catch(noop) stops the cancelled timeout from being treated as an "unhandled error", but now the rejected promise needs to schedule that noop to be executed which triggers the digest. Previously the rejected promise had no handlers so nothing would be scheduled.

@Nickproger Nickproger mentioned this issue Jun 18, 2017
Open
@Narretz
Copy link
Contributor

Narretz commented Jun 19, 2017

@jbedard Thanks for investigating. This looks like the most likely culprit. I have no idea how to fix this, though. Maybe we can special case a noop callback?

@jbedard
Copy link
Collaborator

jbedard commented Jun 19, 2017

I thought of that (special case for noop), but but would be a bit of a breaking change to $q.

Only other thing I've thought of is creating a (private, for now) markExceptionHandled(promise) method.

jbedard added a commit to jbedard/angular.js that referenced this issue Jun 21, 2017
@jbedard
fix($timeout/$interval): do not trigger a digest when cancelling a $t…
df9c54a 
…imeout/$interval

Previously `.catch(noop)` was used on the rejected timeout/interval to prevent an unhandled rejection error. However this would schedule a deferred task to run the `noop`. If the cancelling was outside a digest this could cause a new digest such as with the ng-model `debounce` option.

Fixes angular#16057
jbedard added a commit to jbedard/angular.js that referenced this issue Jun 21, 2017
@jbedard
fix($timeout/$interval): do not trigger a digest on cancel
ec0d810 
Previously `.catch(noop)` was used on the rejected timeout/interval to prevent an unhandled rejection error. However this would schedule a deferred task to run the `noop`. If the cancelling was outside a digest this could cause a new digest such as with the ng-model `debounce` option.

Fixes angular#16057
jbedard added a commit to jbedard/angular.js that referenced this issue Jun 21, 2017
@jbedard
fix($timeout/$interval): do not trigger a digest on cancel
5e58ee5 
Previously `.catch(noop)` was used on the rejected timeout/interval to prevent an unhandled rejection error. However this would schedule a deferred task to run the `noop`. If the cancelling was outside a digest this could cause a new digest such as with the ng-model `debounce` option.

Fixes angular#16057
@jbedard jbedard mentioned this issue Jun 21, 2017
Merged
jbedard added a commit to jbedard/angular.js that referenced this issue Jun 21, 2017
@jbedard
fix($timeout/$interval): do not trigger a digest on cancel
d557ec1 
Previously `.catch(noop)` was used on the rejected timeout/interval to prevent an unhandled rejection error. However this would schedule a deferred task to run the `noop`. If the cancelling was outside a digest this could cause a new digest such as with the ng-model `debounce` option.

Fixes angular#16057
@jbedard jbedard self-assigned this Jun 21, 2017
jbedard added a commit to jbedard/angular.js that referenced this issue Jun 24, 2017
@jbedard
fix($timeout/$interval): do not trigger a digest on cancel
a981a5d 
Previously `.catch(noop)` was used on the rejected timeout/interval to prevent an unhandled rejection error. However this would schedule a deferred task to run the `noop`. If the cancelling was outside a digest this could cause a new digest such as with the ng-model `debounce` option.

Fixes angular#16057
jbedard added a commit to jbedard/angular.js that referenced this issue Jun 28, 2017
@jbedard
fix($timeout/$interval): do not trigger a digest on cancel
12a1ee7 
Previously `.catch(noop)` was used on the rejected timeout/interval to prevent an unhandled rejection error. However this would schedule a deferred task to run the `noop`. If the cancelling was outside a digest this could cause a new digest such as with the ng-model `debounce` option.

Fixes angular#16057
jbedard added a commit to jbedard/angular.js that referenced this issue Jun 30, 2017
@jbedard
fix($timeout/$interval): do not trigger a digest on cancel
1ec77d0 
Previously `.catch(noop)` was used on the rejected timeout/interval to prevent an unhandled rejection error. However this would schedule a deferred task to run the `noop`. If the cancelling was outside a digest this could cause a new digest such as with the ng-model `debounce` option.

Fixes angular#16057
Narretz pushed a commit that referenced this issue Jul 3, 2017
@jbedard @Narretz
fix($timeout/$interval): do not trigger a digest on cancel
cdaa6a9 
Previously, `.catch(noop)` was used on a rejected timeout/interval to prevent an unhandled rejection error (introduced in #c9dffde1cb). However this would schedule a deferred task to run the `noop`. If the cancelling was outside a digest this could cause a new digest such as with the ng-model `debounce` option.

For unit testing, this means that it's no longer necessary to use `$timeout.flush()` when a `$timeout` has been cancelled outside of a digest. Previously, this was necessary to execute the deferred task added by `.catch(noop).
There's an example of such a change in this commit's changeset in the file `/test/ngAnimate/animateCssSpec.js`.

Fixes #16057
Closes #16064
Narretz pushed a commit that referenced this issue Jul 3, 2017
@jbedard @Narretz
fix($timeout/$interval): do not trigger a digest on cancel
a222d0b 
Previously, `.catch(noop)` was used on a rejected timeout/interval to prevent an unhandled rejection error (introduced in #c9dffde1cb). However this would schedule a deferred task to run the `noop`. If the cancelling was outside a digest this could cause a new digest such as with the ng-model `debounce` option.

For unit testing, this means that it's no longer necessary to use `$timeout.flush()` when a `$timeout` has been cancelled outside of a digest. Previously, this was necessary to execute the deferred task added by `.catch(noop).
There's an example of such a change in this commit's changeset in the file `/test/ngAnimate/animateCssSpec.js`.

Fixes #16057
Closes #16064
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@jbedard jbedard

Labels
component: forms
Projects
None yet
Milestone
1.6.x
Development

No branches or pull requests
6 participants
@jbedard @Narretz @Nickproger @masterOL @vpalamarchuk @valenburm