This repository was archived by the owner on Feb 26, 2024. It is now read-only.
fix(proxy): proxyZone should call onHasTask when change delegate, async should call finish only when sync#1030
Merged
mhevery merged 1 commit intoangular:masterfrom Feb 27, 2018
Conversation
4577669 to
49c68b7
Compare
49c68b7 to
108edfb
Compare
f80ee44 to
64b523e
Compare
64b523e to
ea89b35
Compare
mhevery
approved these changes
Feb 27, 2018
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix 4 issues.
finishCallbackwhen the it is pure async.setTimeoutshould not in zone.eventTaskcorrectly.fix angular/angular#22448, which can be described as the following code. (this issue will also bring some other issues), I have created several cases here, https://github.com/angular/angular/pull/22454/files.
I also created test cases in
zone.jsto simulate@angular/core/async.the case above will report
Error: 'expect' was used when there was no current spec.Because
simple timeouttest will not wait forsetTimeoutfinished.There are several reasons.
onHasTaskwhen change delegate, otherwise the new delegateZoneSpec may never getonHasTasktriggered.For example:
proxyZonewith delegatezoneSpec1.setTimeoutin zoneSpec1zoneSpec2.setTimeoutagainThe onHasTask will not triggered in
zoneSpec2when callsetTimeoutbecause thetaskCountis already be1which was increased byzoneSpec1.when we fix the first issue, the case will still fail, because jasmine will call
setTimeoutto monitor timeout, and thissetTimeoutwill also run inProxy(AsyncTest)Zone, so there will always apendingMacroTaskto make the test case timeout forever.When we fix the above 2 issues, there will come out a new problem, https://github.com/angular/zone.js/blob/master/lib/zone-spec/async-test.ts#L84, the
finishCallbackwill triggered twice (one byonHasTask, one byonInvoke). We should only trigger theonInvokeone when thecallbackis puresync.When we fix all those 3 problems, there still one left in
angularside, https://github.com/angular/angular/blob/master/packages/core/testing/src/async.ts#L91,asyncshould reset thedelegateSpecoutside ofcurrentZone.run.I have created several test cases with
asyncof@angular/core, and I will also create test cases inzone.jsside.@mhevery , please review this one, it seems will bring some impact of
AsyncTestZoneandasyncin angular, thank you!