Skip to content

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

Merged
mhevery merged 1 commit into
angular:masterfrom
thelgevold:fix-digest-in-progress
Jun 7, 2016
Merged

mhevery merged 1 commit into
angular:masterfrom
thelgevold:fix-digest-in-progress

Conversation

@thelgevold

Copy link
Copy Markdown
Contributor

**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
@thelgevold

Copy link
Copy Markdown
Contributor Author

@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

andreialecu commented Jun 7, 2016 •

Copy link
Copy Markdown
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? :)

👍

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

marcalj commented Jun 8, 2016

Copy link
Copy Markdown

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

@angular-automatic-lock-bot

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants