Skip to content

feat(cdk): implement waitForTasksOutsideAngular in ProtractorHarnessEnvironment#18361

Closed
LayZeeDK wants to merge 1 commit intoangular:masterfrom
LayZeeDK:LayZeeDK/feat/protractor-harness-environment-wait-for-tasks-outside-angular
Closed

feat(cdk): implement waitForTasksOutsideAngular in ProtractorHarnessEnvironment#18361
LayZeeDK wants to merge 1 commit intoangular:masterfrom
LayZeeDK:LayZeeDK/feat/protractor-harness-environment-wait-for-tasks-outside-angular

Conversation

@LayZeeDK
Copy link
Contributor

@LayZeeDK LayZeeDK commented Feb 1, 2020

Implement waitForTasksOutsideAngular in ProtractorHarnessEnvironment.

Fixes #17412 for Protractor.

How do we best test this? I see some CDK and Material directives and components using setTimeout and requestAnimationFrame.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Feb 1, 2020
// https://github.com/angular/components/issues/17412
await browser.executeAsyncScript(
'var callback = arguments[arguments.length - 1];'
+ 'requestAnimationFrame(callback);');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to spend more time thinking about this. e.g. requestAnimationFrame does not wait for macro tasks (as our task interceptor does in the testbed environment).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't? My first attempt used setTimout, but I thought rAF waited for both micro and macro task queues to clear.

Copy link
Contributor Author

@LayZeeDK LayZeeDK Feb 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If so, we could call the async script callback only after setTimeout(..., 1) and requestAnimationFrame have resolved.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should be able to use a similar strategy to what we did for the TestbedHarnessEnvironment, we'll just need to run the task interceptor as a browser script

Copy link
Member

@devversion devversion Feb 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LayZeeDK I might miss something here, but won't things like setTimeout and requestAnimationFrame never work here because we don't know what durations the macro tasks take? e.g. the component could have something like setTimeout(..., 300) inside.

This is why we need ZoneJS here since it can capture that information for us. I think what @mmalerba proposed seems right. I'm unsure how it will work yet, but it should be possible.

Copy link
Contributor Author

@LayZeeDK LayZeeDK Feb 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, so you want to patch the environment with zones or similar like the TestBed harness environment.

Two issues which might be an entirely separate discussion of their own:

  1. With ES2017+ bundles, it's not possible to intercept async-await.
  2. Can we support applications using NoopNgZone with the task interceptor?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for this PR we should just focus on getting protractor working with es2015 and using NgZone. That's what we currently support for the TestbedHarnessEnvironment.

  1. Yeah, that is a separate issue that affects many parts of Angular, it is being tracked here: angular with tsconfig target ES2017 async/await will not work with zone.js angular#31730
  2. Currently we have not tested it with NoopNgZone and my guess is it won't work. If you have thoughts/ideas about this please feel free to open a new issue for the feature and discuss them there.

@LayZeeDK
Copy link
Contributor Author

LayZeeDK commented Feb 3, 2020

Is v9 going to be released without this implemented?

@mmalerba
Copy link
Contributor

mmalerba commented Feb 3, 2020

Is v9 going to be released without this implemented?

At this point, most likely yes, this will have to go in a follow-up patch release

@LayZeeDK
Copy link
Contributor Author

LayZeeDK commented Feb 3, 2020

Thanks for answering. How do we proceed from here? Do I close this PR? Open an issue? I don't have any immediate thoughts on baking the task interceptor into a Protractor environment.

@mmalerba
Copy link
Contributor

mmalerba commented Feb 3, 2020

Ok, I'll close this PR. I created an issue to track this here: #18375

@mmalerba mmalerba closed this Feb 3, 2020
@angular-automatic-lock-bot
Copy link

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 Mar 5, 2020
@LayZeeDK LayZeeDK deleted the LayZeeDK/feat/protractor-harness-environment-wait-for-tasks-outside-angular branch December 1, 2022 15:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes PR author has agreed to Google's Contributor License Agreement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(cdk/testing): ComponentHarness doesn't wait for tasks outside NgZone

4 participants