syndesisio / syndesis-ui

The front end application or UI for Syndesis - a flexible, customizable, cloud-hosted platform that provides core integration capabilities as a service. It leverages Red Hat's existing product architecture using OpenShift Online/Dedicated and Fuse Integration Services.
https://syndesis.io/
14 stars 28 forks source link

Run observable outside of angular zone #209

Closed jludvice closed 7 years ago

jludvice commented 7 years ago

Why? see this article https://christianliebel.com/2016/11/angular-2-protractor-timeout-heres-fix/

It breaks protractor tests.

jludvice commented 7 years ago

@gashcrumb I tried to use ngZone in app initializer but I don't know how to properly inject ngZone :(

With this commit I'm getting error in browser console:

Error: Uncaught (in promise): TypeError: Cannot read property 'runOutsideAngular' of undefined
TypeError: Cannot read property 'runOutsideAngular' of undefined

I found usage of initializer only here and I don't know how to make angular inject NgZone

  providers: [
    {
      provide: APP_INITIALIZER,
      useFactory: appInitializer,
gashcrumb commented 7 years ago

Think you need to add ngZone to this list ->

https://github.com/redhat-ipaas/ipaas-ui/blob/master/src/app/app.module.ts#L158

You'll need to add an NgZone import to this line ->

https://github.com/redhat-ipaas/ipaas-ui/blob/master/src/app/app.module.ts#L2

like this example -> https://angular.io/docs/ts/latest/api/core/index/NgZone-class.html

and then this is where you actually inject ngZone: NgZone ->

https://github.com/redhat-ipaas/ipaas-ui/blob/master/src/app/app.module.ts#L25

dsimansk commented 7 years ago

Thanks @gashcrumb, I got it working locally. Josef will wrap it up and submit PR in a few minutes (no pressure :)).

jludvice commented 7 years ago

Hello @gashcrumb thanks for explanation. I was missing just this last step

  providers: [
    {
      provide: APP_INITIALIZER,
      useFactory: appInitializer,
      deps: [ConfigService, OAuthService, UserService, NgZone], //  <-- here
      multi: true,
    },

My original commit is gone (it was based on my branch with tests), this one is based on master branch. Can you review if it works correctly ?

@jimmidyson @kahboom Just a note for the future - can we default to ngZone.runOutsideAngular(() in case we need timeout or observable?

Protractor waiting worked correctly for me with this fix.

jludvice commented 7 years ago

Resolved by #219

kahboom commented 7 years ago

Ty @jludvice !