Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Angular ChangeDetection doesn't work #52

Closed
kzimny opened this issue Apr 24, 2020 · 26 comments
Closed

Angular ChangeDetection doesn't work #52

kzimny opened this issue Apr 24, 2020 · 26 comments

Comments

@kzimny
Copy link

kzimny commented Apr 24, 2020

Hi,
does anybody know why the angular ChangeDetection is not working with cordova-plugin-wkwebview-file-xhr? I load data in my angular 9+ project and get the correct response from api but the view can not be refreshed.

import { ChangeDetectionStrategy, ChangeDetectorRef, Component, OnInit } from '@angular/core';

import { ICaseLibrary } from '@shared/model';
import { AdminService } from '../admin.service';

@Component({
    selector: 'app-caselibrary',
    templateUrl: './caselibrary.template.html',
    changeDetection: ChangeDetectionStrategy.OnPush,
})
export class CaseLibraryComponent implements OnInit {
    casesFromDB: ICaseLibrary[];

    constructor(
        private adminService: AdminService,
        private cdRef: ChangeDetectorRef) { }

    ngOnInit(): void {
        this.adminService.getCasesForCaseLibrary().subscribe(
            response => {
                this.casesFromDB = response;
                this.cdRef.markForCheck(); // <== does not work in cordova using the WKWebView
            });
    }
}

As soon as this.cdRef.markForCheck(); is run in NgZone the view is going refreshed:

            this.ngZone.run(() => {   
                this.cdRef.markForCheck();
            });

Does cordova-plugin-wkwebview-file-xhr provides its own promise polyfill causing the angular change detection to break? Do I miss something? How to get the angular change detection work without run markForCheck() in NgZone? My angular code is shared between web, desktop and mobile platforms and the change detection should not be run outside of angular zone. Any idea?

@nicearma
Copy link

nicearma commented May 5, 2020

Same problem here, using ionic 4

@manish2788
Copy link
Member

@kzimny Let me tru to reproduce this issue. Will get back to you.

@kzimny
Copy link
Author

kzimny commented May 9, 2020

@manish2788 yes, please fix the issue asap. With the ngZone.run(...) I found a temporary workaround. When I use cordova-plugin-wkwebview without cordova-plugin-wkwebview-file-xhr there is no issue with angular change detection. As soon as cordova-plugin-wkwebview-file-xhr is installed, change detection doesn't work as expected. cordova-plugin-wkwebview cannot deal with http cookies, that the reason why I installed cordova-plugin-wkwebview-file-xhr. What causes the misbehavior? There are no errors or warnings in the console.

@nicearma
Copy link

nicearma commented May 9, 2020

I think the problem is about the plugin change the reference of XMLHttpRequest and because of this the code added/modify with ngZone is put it out.
I think to solve this problem we have have to see what ngZone work with XMLHttpRequest and ssee what is missing.

@kzimny
Copy link
Author

kzimny commented May 9, 2020

@nicearma ngZone is a temporary workaround.

have to see what ngZone work with XMLHttpRequest

No, cordova-plugin-wkwebview-file-xhr should work without calling ngZone.run(...).

@nicearma
Copy link

nicearma commented May 10, 2020

I din't make reference to your code, you can see at https://github.com/angular/angular/search?q=XMLHttpRequest&unscoped_q=XMLHttpRequest that ngZone use the native XMLHttpRequest, so i think the solution at this problem is study the ngZone repository and see the interaction with XMLHttpRequest and compare with the custom XMLHttpRequest.

ngZone add the magic around all kind of JS code (xhr, rxjs, etc - you can see in the repository), normally in angular you dont need to do this.

this.adminService.getCasesForCaseLibrary().subscribe(
            response => {
                this.casesFromDB = response;
                **this.cdRef.markForCheck();** // <== Normally Angular doesn't need this, only if you use functions that ngZone doesn't add the magic, like one special async function, normally all rxjs functions have the ngZone detectors
            });

I think one simple test could be to add the js this plugin before the ngZone JS and see if work like it should

You can see more about the interaction with ngZone here See https://github.com/angular/angular/blob/698b0288bee60b8c5926148b79b5b93f454098db/packages/zone.js/lib/browser/browser.ts#L95

@kzimny
Copy link
Author

kzimny commented May 11, 2020

Thank you for the hints. If you use changeDetection: ChangeDetectionStrategy.OnPush, in your component this.cdRef.markForCheck(); is needed to refresh the view.

@ygodin
Copy link

ygodin commented May 15, 2020

I'm having the same issue with Angular 9. Forcing refresh by interacting with the app will trigger an update. It would be very nice to have a fix, this plugin will saves us a lot of trouble.

@kzimny
Copy link
Author

kzimny commented May 19, 2020

@manish2788 any chance to get the plugin work with angular ChangeDetection?

@manish2788
Copy link
Member

Hi @kzimny,

Will it be possible for you to share a sample angular app which demonstrates the erratic behaviour pointed by you.

@ygodin
Copy link

ygodin commented May 25, 2020

Hi @manish2788 , @kzimny here's a demo app that expose the problem, i've added the necessary to the readme to be able to setup and start both web and cordova version so you can compare. The Angular build has sourcemap enabled, app.component.ts is the one that trigger the call and should refresh it's view at the same time.

https://github.com/ygodin/cordova-angular-refresh-problem

@kzimny
Copy link
Author

kzimny commented May 26, 2020

Thank you. Your example should work on android in windows environment but it doesn't in my case. When I start cordova run android the app loads. The message LOADING does not disappear, but it should on android. npm run web doesn't work on windows. I get the message Cannot GET /. If the oracle guys work on windows your example will probably not work for them.

@ygodin
Copy link

ygodin commented May 26, 2020

@kzimny the refresh problem happen only on IOS, this plugin doesn't replace xmlhttprequest on Android so no refresh problem. Not the point of this issue i think, but after adding the android platform (not included in the setup.sh) it works fine.

For the web version it might have something to do with the base path, i'll check.

@kzimny
Copy link
Author

kzimny commented May 26, 2020

@ygodin agree that the problem does not exist on android platform. But that's the point. Your example doesn't work on android on my side. The message LOADING does not disapper on android running on windows environment. Hope it works for oracle guys.

@ygodin
Copy link

ygodin commented May 26, 2020

@kzimny Do you see any error in the debugger ? I'm using Cordova 8.x.x

@kzimny
Copy link
Author

kzimny commented May 26, 2020

Cordova 9.0.1, I found that it works fine on real device bot not on android emulator. Maybe something is wrong with my emulator settings. Anyway, android was newer a problem. The problem exist on iOS. Hope @manish2788 can find the solution soon.

@ygodin
Copy link

ygodin commented May 26, 2020

@kzimny i just pushed a fix for the windows web version, the baseDir had to be changed so it works on both platform.

@ygodin
Copy link

ygodin commented May 26, 2020

In the meatime, here's how we fixed it, we load this service at the root level of our app, this catch everything, not only HTTPClient. From what i see it looks like a race condition between the plugin polyfill and zone, both trying to patch xmlhttprequest.

@Injectable()
export class XmlHttpRequestWkwebviewPatcher {
  constructor(
    private readonly appRef: ApplicationRef,
    @Inject(WINDOW) private readonly window: Window,
    @Inject(DOCUMENT) private readonly document: Document
  ) {
    if (window.cordova && document.URL.includes('ionic://')) {
      const origOpen = XMLHttpRequest.prototype.open;
      XMLHttpRequest.prototype.open = function() {
        this.addEventListener(
          'load',
          () => {
            setTimeout(() => {
              appRef.tick();
            });
          },
          {once: true}
        );
        origOpen.apply(this, arguments);
      };
    }
  }
}

@manish2788
Copy link
Member

Thanks for sharing the app and relevant informations. Will get back upon this.

@sanket-bhalerao
Copy link

In the meatime, here's how we fixed it, we load this service at the root level of our app, this catch everything, not only HTTPClient. From what i see it looks like a race condition between the plugin polyfill and zone, both trying to patch xmlhttprequest.

@Injectable()
export class XmlHttpRequestWkwebviewPatcher {
  constructor(
    private readonly appRef: ApplicationRef,
    @Inject(WINDOW) private readonly window: Window,
    @Inject(DOCUMENT) private readonly document: Document
  ) {
    if (window.cordova && document.URL.includes('ionic://')) {
      const origOpen = XMLHttpRequest.prototype.open;
      XMLHttpRequest.prototype.open = function() {
        this.addEventListener(
          'load',
          () => {
            setTimeout(() => {
              appRef.tick();
            });
          },
          {once: true}
        );
        origOpen.apply(this, arguments);
      };
    }
  }
}

@ygodin can you specify any details on how to use/load this service?

@sanket-bhalerao
Copy link

Thanks for sharing the app and relevant information. Will get back upon this.

@manish2788 is there any progress on this?

@manish2788
Copy link
Member

@sanket-bhalerao Hi Sanket, We are working on the reported issues. Next week we be sharing the updates.

@sanket-bhalerao
Copy link

@manish2788 any updates on this?

@manish2788
Copy link
Member

@sanket-bhalerao We have been verifying the sample app provided by @ygodin.
@ygodin I was not able to replicate the behaviour you mentioned in Read Me file of your repo. Does your repo contains any changes on top of our plugin to resolve the issue.

@DennisHerr
Copy link

DennisHerr commented Aug 18, 2020

so there is a simple fix for this. ✔️
the only thing which has to be done is applying a monkey patch to RXJs.

so I have done it inside my HTTP Interceptor for all requests.

// your imports

 @Injectable()
 export class RequestInterceptor implements HttpInterceptor {
 
    runInThisZone(zone) {
     return function patchedobsverable(source) {
       return Observable.create(observer => {
        const onNext = (value) => zone.run(() => observer.next(value));
         const onError = (e) => zone.run(() => observer.error(e));
        const onComplete = () => zone.run(() => observer.complete());
        return source.subscribe(onNext, onError, onComplete);
       });
     };
  }

constructor(private zone: NgZone) {}

intercept(req: HttpRequest < any > , next: HttpHandler): Observable < HttpEvent < any >> {
    if (window.usingCordova) { // your cordova/platform check here
        req = req.clone();
        return this.zone.run(() => next.handle(req).pipe(this.runInThisZone(this.zone)));
    } else {
        return next.handle(req);
    }
  }
}

so there we are. all request are forced into the right zone and the angular change detection triggers correctly. 😄

@manish2788
Copy link
Member

Plugin doesn't support out of the box fix for Angular. Closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants