oracle / cordova-plugin-wkwebview-file-xhr

Cordova Plugin for WebView File XHR
Universal Permissive License v1.0
138 stars 120 forks source link

Angular ChangeDetection doesn't work #52

Closed kzimny closed 3 years ago

kzimny commented 4 years ago

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 commented 4 years ago

Same problem here, using ionic 4

manish2788 commented 4 years ago

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

kzimny commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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

ygodin commented 4 years ago

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 commented 4 years ago

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

manish2788 commented 4 years ago

Hi @kzimny,

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

ygodin commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

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

kzimny commented 4 years ago

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 commented 4 years ago

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

ygodin commented 4 years ago

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 commented 4 years ago

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

sanket-bhalerao commented 4 years ago

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 commented 4 years ago

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

@manish2788 is there any progress on this?

manish2788 commented 4 years ago

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

sanket-bhalerao commented 4 years ago

@manish2788 any updates on this?

manish2788 commented 4 years ago

@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 commented 4 years ago

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 commented 3 years ago

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