feat(cdk/testing): add method to wait for async tasks outside angular to complete

[devversion]

Adds a new method to the ComponentHarness base class that can be
used to wait for async tasks to complete. This is useful for harness
authors as sometimes the component schedules tasks outside of the
Angular zone which therefore won't be captured by whenStable.

These tasks can be relevant for the harness logic though, so there
should be an option for harness authors to await all async tasks
(not only the ones captured in the ng zone).

e.g. the slider re-adjusts when the directionality changes. This
re-rendering happens in the next tick. To make sure that setValue
works when the directionality changed, we need to wait for the async
tasks to complete, so that the slider completed re-rendering and the
value can be changed through a mouse click.

[mmalerba]

This looks good to me, just a naming suggestion. We should see what others think for the name too

[jelbourn]

LGTM aside from the naming discussion

[mmalerba]

@devversion Inside google3 it appears that the actual typings from the angular/angular repo (packages/zone.js/lib/zone.d.ts) are present. That's causing the compiler to complain that you're re-declaring them in this PR. Is there a way that we can depend on the real typings?

@jelbourn If not, how do you think we should handle this? just copybara out the import of the .d.ts file created in this PR?

[devversion]

@mmalerba I had logic to access the zone.js types, but it actually caused the whole cdk compilation unit (in legacy gulp) to break because zone types brought in the node types. I tried excluding them somehow, but wasn't able to get it work.

[jelbourn]

Omitting those type from the sync seems pretty trivial

@devversion for the naming, I think that waitForTasksOutsideAngular does explain what the function does from the perspective of the people using it. The majority of people don't know much about zones, with some knowing enough to do runOutsideAngular, so I think this will align with people's idea of what's happening

[devversion]

@jelbourn We said in the meeting that the case that someone uses runOutsideAngular is uncommon, hence explicitly running tasks outside Angular is probably a bit more advanced and involves a basic understanding of zones/how CD works.

I'm not in particular saying we should mention zone in the name, nor do I say that my initial proposal is the best. I totally see why you want it to be named waitForTasksOutsideAngular. Though I personally disagree that the name is semantically correct. It's describing only a subset of the action that is performed by calling this method (hence doing more than expected.. which can result in unexpected behavior in cases we did not think of). This is just my thinking, and its good that we discuss it. I think both names have trade-offs, so I'm fine with what you/Miles prefer 😄

[mmalerba]

@devversion I think one thing that kind of mitigates your concern is the fact that waiting for the NgZone always happens, even if you don't call this method. So yes, while you're technically right, the only difference a user will notice is the fact that it waits for stuff outside the NgZone too, if they do call this method

[devversion]

@mmalerba But not necessarily. In more advanced harnesses where the provided default platform-agnostic TestElement is not sufficient and the platform is accessed directly, the NgZone stabilizing does not run automatically. In those cases, the method will do more than expected and rather confuse people on why other tasks are awaited too.

As said, I see your reasoning. It will be helpful for most cases, but I personally prefer method names to not be based on their effect when called in specific scenarios, but rather based on what is done regardless of the callee context. Not trying to argue on the name, just trying to explain my standpoint.

[mmalerba]

I don't think people should ever be accessing the platform directly in harness code. The harnesses are meant to be platform-independent. I'm sure you're right that someone will try to do that, but its not really a use case I think we need to be concerned about supporting

[devversion]

Fair point. My thinking was that harnesses can be also built just for one platform though (i.e. with the goal of just making breaking changes less painful in tests / or just making test authoring easier).
Above was just one example (who know what other scenarios there are). I guess it's more of a best-practice question for me. My thinking is probably just different on how method names should be called. Anyway, I'm good with that name as it will work in most of the cases just fine.

[mmalerba]

@devversion It looks like ProxyZoneStatic and ProxyZone are not part of the real zone.js d.ts file. Can you move these to a separate file (either task-state-zone-interceptor.ts or some new file like proxy-zone-types.d.ts). I need to be able to easily strip away just the ones that are part of the real zone.js d.ts file for google3.

[devversion]

Makes sense. I've split the ProxyZone types into a separate file.

[angular-automatic-lock-bot]

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.}