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

fix(ngUpgrade): prevent digest already in progress #9054

Merged
merged 1 commit into from Jun 7, 2016

Conversation

Projects
None yet
5 participants
@thelgevold
Contributor

thelgevold commented Jun 7, 2016

**This is a resubmit of #9046

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

  • [x ] Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)
Under certain conditions ngUpgrade will throw "digest already in progress" errors. Seems to be related to overlapping digest cycles between ngUpgrade and the Angular 1 application.
Repro here:
#8951

What is the new behavior?
By using $evalAsync, digest cycles will no longer overlap since Angular will check if a digest cycle is already in progress before triggering a new one. If a digest is already a progress the ngUpgrade update will just join the existing cycle.

Does this PR introduce a breaking change?

  • Yes
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

fix(ngUpgrade): prevent digest already in progress
fix(ngUpgrade): prevent digest already in progress

@googlebot googlebot added the cla: yes label Jun 7, 2016

@thelgevold

This comment has been minimized.

Show comment
Hide comment
@thelgevold

thelgevold Jun 7, 2016

Contributor

@mhevery @chuckjaz
Sorry about the failing test in my previous PR. I am resubmitting with a fix.

Basically I switched from $applyAsync to $evalAsync. This passes the tests and still fixes the original issue.

Contributor

thelgevold commented Jun 7, 2016

@mhevery @chuckjaz
Sorry about the failing test in my previous PR. I am resubmitting with a fix.

Basically I switched from $applyAsync to $evalAsync. This passes the tests and still fixes the original issue.

@andreialecu

This comment has been minimized.

Show comment
Hide comment
@andreialecu

andreialecu Jun 7, 2016

Contributor

I have the same issue and just tried this PR.

Interestingly, I have a fairly complex angular1+angular2 hybrid app and this one liner not only fixed the problem but also seems to have helped performance a lot!

I guess there were a lot of digests happening in very close succession triggered by this which are now better consolidated.

LGTM pls? :)

👍

Contributor

andreialecu commented Jun 7, 2016

I have the same issue and just tried this PR.

Interestingly, I have a fairly complex angular1+angular2 hybrid app and this one liner not only fixed the problem but also seems to have helped performance a lot!

I guess there were a lot of digests happening in very close succession triggered by this which are now better consolidated.

LGTM pls? :)

👍

@mhevery mhevery merged commit 7cefec7 into angular:master Jun 7, 2016

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@marcalj

This comment has been minimized.

Show comment
Hide comment
@marcalj

marcalj Jun 8, 2016

This will land in rc2? It's about to be released??? pleeeeease :)

marcalj commented Jun 8, 2016

This will land in rc2? It's about to be released??? pleeeeease :)

@thelgevold thelgevold referenced this pull request Jun 8, 2016

Closed

ngUpgrade not managing digest cycles properly #8951

1 of 3 tasks complete

tycho01 added a commit to tycho01/angular that referenced this pull request Jun 10, 2016

fix(ngUpgrade): prevent digest already in progress (#9054)
fix(ngUpgrade): prevent digest already in progress

tycho01 added a commit to tycho01/angular that referenced this pull request Jun 21, 2016

fix(ngUpgrade): prevent digest already in progress (#9054)
fix(ngUpgrade): prevent digest already in progress

@gkalpak gkalpak referenced this pull request Jul 4, 2017

Closed

fix(upgrade): prevent digest in progress errors #17692

1 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment