bug: Angular application (Zone.js) + cordova.js file #2896

AlanThiec · 2020-05-09T12:33:28Z

Bug Report

This issue can interest also https://github.com/ionic-team/ionic

Capacitor Version

Capacitor 1.x and 2.x

Affected Platform(s)

Android
iOS
Electron
Web

Current Behavior

Angular application (with or whithout Ionic Angular) is broken by the "cordova.js" injected in head scripts of index.html. This can create several problem as described in the following issue from the Angular repository.
See: angular/angular#22509

In my case, when I'm in a several "router-oulet" deep and hit the back button, the previous child component will be created but not rendered correctly (no life cycle hooks are called like ngOnInit).

Expected Behavior

In my case, the previous child should be created and rendered correctly (life cycle hooks should be called).

Sample Code or Sample Application Repo

https://github.com/AlanRaz/capacitor-angular-issue

Reproduction Steps

npm i && npm run build && npx cap sync android && npx cap open android

Follow the navigation button (in green) then when you arrive to the step 2, just hit the back button (in red).
You can see that the component are no more rendered correctly and see in the console log that the constructor is called but the ngOnInit are not called.

Other Technical Details

This is also a problem with older Angular versions.
See: angular/angular#22509

Other Information

It took me a while to found this issue because I didn't know that Capacitor added the cordova.js at run time in the header of the index.html.

What we use to do in old Cordova/Angular app to resolve this was to follow (https://github.com/angular/angular/blob/master/packages/zone.js/NON-STANDARD-APIS.md#others) and put the scripts in this order:

<script src="zone.js"></script>
<script src="cordova.js"></script>
<script src="zone-patch-cordova.js"></script>

I don't know witch solutions will be good in termes of design and maintainability.

Let, the user to add a workaround like this one in the head of index.html:

<script>
  window.addEventListener = function () {
    (window.EventTarget || Window).prototype.addEventListener.apply(this, arguments);
  };
  window.removeEventListener = function () {
    (window.EventTarget || Window).prototype.removeEventListener.apply(this, arguments);
  };
  document.addEventListener = function () {
    (window.EventTarget || Document).prototype.addEventListener.apply(this, arguments);
  };
  document.removeEventListener = function () {
    (window.EventTarget || Document).prototype.removeEventListener.apply(this, arguments);
  };
</script>

But, I think it's really not explicit for the end-user.

Capacitor should inject directly this workaround before injecting cordova.js.
I don't know if this workaround can have side effect on other web libraries (or other libraries to have the same problem).

The text was updated successfully, but these errors were encountered:

AlanThiec · 2020-05-28T13:10:08Z

Any thoughts on this ?

imhoffd · 2020-06-23T03:00:39Z

Yes, for Cordova compatibility Capacitor generates a file at the top of <head>. One of the files included is cordova.js if Cordova plugins are detected. I doubt the injected Capacitor scripts need to be at the very top of <head>.

In modern Angular apps, it appears zone.js is bundled into the vendor bundle. cordova.js is usually added manually to <head> in Angular apps, but it appears the bundles are loaded at the bottom of <body>, so how is this not an issue for Angular w/ Cordova?

AlanThiec · 2020-06-30T08:41:25Z

It's also a problem with Angular and Cordova project, what I usually do is changing the import order (but I don't know if it's the correct thing to do to fix this kind of problem).

I'm pretty sure I can reproduce the problem with an Ionic project too.

KevinKelchen · 2021-02-24T21:35:28Z

We're experiencing a very similar issue in our app that we're in the middle of migrating from Cordova to Capacitor.

My npx ionic info:

Ionic:

Ionic CLI : 6.12.3
Ionic Framework : @ionic/angular 5.2.3
@angular-devkit/build-angular : 0.1000.1
@angular-devkit/schematics : 10.0.3
@angular/cli : 10.0.1
@ionic/angular-toolkit : 2.3.0

Capacitor:

Capacitor CLI : 2.4.5
@capacitor/core : 2.4.5

Utility:

cordova-res : not installed
native-run : 1.3.0

System:

NodeJS : v12.18.0 (/usr/local/bin/node)
npm : 6.14.4
OS : macOS Catalina

In certain cases it seems like Angular change detection is not running when it should. It's not an issue using Cordova or a Web deployment.

Using the basic workaround in this blogpost (also effectively referenced in the OP) seems to fix the issue. My investigation so far corroborates the blogpost. Here's my working theory at this point:

Ionic Angular with Cordova

Using Ionic Angular with Cordova the script load order is the following from the generated index.html in the www directory using a debug npx ionic cordova build ... command:

<body>
  <app-root></app-root>
  <script src="runtime.js" defer></script>
  <script src="polyfills.js" defer></script>
  <script src="styles.js" defer></script>
  <script src="cordova.js" defer></script>
  <script src="vendor.js" defer></script>
  <script src="main.js" defer></script>
</body>

Zone.js is present in polyfills.js which means it is loaded prior to cordova.js.

Without any JavaScript patching, window and document share the addEventListener and removeEventListener functions via EventTarget.prototype (running window.addEventListener === EventTarget.prototype.addEventListener equals true). Zone.js patches the EventTarget addEventListener and removeEventListener APIs and therefore effectively patches them on window and document via a shared prototype reference which is not a problem.

cordova.js then comes along and saves off references to the original implementations of the window and document addEventListener and removeEventListener functions. It then reassigns/clobbers the window and document functions but does not touch EventTarget. This means Zone.js patching still exists on EventTarget. However, the prototype reference to EventTarget is broken due to the reassignment/clobbering. While this could be a problem for other third-party libraries that are loaded later that wish to patch the window and document handlers via EventTarget (such as Sentry, which we also use), this doesn't end up being a problem for Zone.js because when cordova.js saved off the window and document functions they were already patched by Zone.js.

Ionic Angular with Capacitor

As noted earlier in this thread, with Capacitor the content of cordova.js is injected into the document at the top of the <head> element instead of being a separate <script> in the <body> element. polyfills.js (which includes Zone.js) is still loaded in the <body> element as depicted in the code snippet above. Therefore, the load order has changed.

Since the load order has changed, cordova.js reassigns/clobbers the window and document event handler functions before Zone.js gets a chance to patch them.

A similar issue that was a result of the cordova.js/Zone.js load order change can be found here.

Solutions?

That cordova.js clobbers those APIs making it difficult for libraries loaded later to work as expected seems somewhat flawed. I know that's a Cordova issue and not a Capacitor issue but it still felt worth expressing.

As I mentioned above, the solution in the referenced blogpost worked for me while doing local debugging. Specifically, it's adding this code before cordova.js loads or before cordova.js stores off the existing event listener implementations:

window.addEventListener = function () {
  EventTarget.prototype.addEventListener.apply(this, arguments);
};

window.removeEventListener = function () {
  EventTarget.prototype.removeEventListener.apply(this, arguments);
};

document.addEventListener = function () {
  EventTarget.prototype.addEventListener.apply(this, arguments);
};

document.removeEventListener = function () {
  EventTarget.prototype.removeEventListener.apply(this, arguments);
};

My understanding is that this workaround makes it so that even when cordova.js reassigns the window and document events, the references to the existing implementations it stores off still have a linkage to the EventTarget implementations by virtue of calling them explicitly. These EventTarget implementations are still patched by Zone.js so change detection works as expected. Other third-party libraries like Sentry which also patches EventTarget might likewise benefit since EventTarget is still used.

For the particular change detection problem I'm seeing in our app, I seemingly only needed the window events but I would include the document events as well since the same type of issue can happen with them (and these change detection issues can be subtle).

The challenge is having a way to get this code to run before cordova.js is executed because Capacitor appears to inject the contents of cordova.js into the document before any user-defined JavaScript can run.

While the particular change detection problem in our app can be "fixed" by adding the workaround after cordova.js is executed, it undoes what patching cordova.js does which feels rather dangerous when looking at the cordova.js code and considering what Cordova plugins might be expecting.

At this point I'm considering using https://www.npmjs.com/package/patch-package to add the workaround to cordova.js. I've never used patch-package before but in preliminary testing locally it seems to work as expected. This is what the patch file contents looks like for patching cordova.js prior to the window and document event listener functions being stored off:

diff --git a/node_modules/@capacitor/core/cordova.js b/node_modules/@capacitor/core/cordova.js
index a5418fb..e269862 100644
--- a/node_modules/@capacitor/core/cordova.js
+++ b/node_modules/@capacitor/core/cordova.js
@@ -105,6 +105,23 @@
          * Intercept calls to addEventListener + removeEventListener and handle deviceready,
          * resume, and pause events.
          */
+        // BEGIN PATCH for https://github.com/ionic-team/capacitor/issues/2896#issuecomment-785400150
+        window.addEventListener = function () {
+          EventTarget.prototype.addEventListener.apply(this, arguments);
+        };
+
+        window.removeEventListener = function () {
+          EventTarget.prototype.removeEventListener.apply(this, arguments);
+        };
+
+        document.addEventListener = function () {
+          EventTarget.prototype.addEventListener.apply(this, arguments);
+        };
+
+        document.removeEventListener = function () {
+          EventTarget.prototype.removeEventListener.apply(this, arguments);
+        };
+        // END PATCH
         var m_document_addEventListener = document.addEventListener;
         var m_document_removeEventListener = document.removeEventListener;
         var m_window_addEventListener = window.addEventListener;

However, that means our app will work differently than almost all other Capacitor apps and who knows what issues could arise over time that are specific to our "fork." Also, we really don't want to maintain a package patch going forward.

It would be awesome if the workaround was added to Capacitor or if there was a clean, first-party way to configure this. I know that'd take some time and would probably be a v3 item (we're on v2 right now), so hopefully I can come up with a way to work around the issue now and at some point be able to drop the workaround.

If there's interest in a PR to implement the workaround I've shown above I will happily create one.

Thank you so much!

Kevin

Ventzy · 2022-10-29T15:32:40Z

@KevinKelchen Did you find any safe workaround of the issue? On a first glance, adding the patch script in the solved my immediate problem, however, as pointed in your great overview above it may break cordova plugins in an unexpected ways as it is getting executed after cordova js.

KevinKelchen · 2022-10-29T19:18:34Z

@KevinKelchen Did you find any safe workaround of the issue? On a first glance, adding the patch script in the solved my immediate problem, however, as pointed in your great overview above it may break cordova plugins in an unexpected ways as it is getting executed after cordova js.

Hi Ventzy, 👋

Thank you for the kind remark!

We went ahead with the patch as described in my message and have been running with it for the last 1.5 years in production in a non-trivial Angular app.

We have not found any issues using the workaround thus far. 🤞

It'd of course be preferable for the change to be made to Capacitor itself. However, in the meantime and based on our experience, I'd hope that the workaround would be solid for you.

Take care, and hope all goes well,

Kevin

matafokka · 2023-01-06T19:27:28Z

Hello and happy new year! Chiming in from ionic-team/ionic-framework#26566

I've encountered a related problem. ~~The provided workaround worked just fine for the simple app. I hope I'll be able to test it in a real app with both Capacitor and Cordova plugins.~~

UPD: it breaks Platform.ready() on Android using both Ionic 5 and 6. Example code:

test = false;

constructor(private platform: Platform) {
  // Platform.ready() promise will never resolve, thus:
  this.platform.ready().then(() => {
    console.log("Platform ready"); // This will never show up in logcat
    this.test = true; // This will stay false
  });
}

Since our app broke from the start, we've decided stay with workarounds I've provided below. Just to be safe :)

I also would like to provide safer, though requiring a bit of boilerplate, workarounds. I hope they will work for you as well.

For additional info, see ionic-team/ionic-framework#26566

Problem

Changes in @HostListener() on document:click, document:pointerdown and document:click (probably, more events are affected) are not processed by Angular change detection in an Android app (while working in a browser on both PC and Android).

Example code:

@HostListener("document:click")
onDocumentClick() {
  this.documentClickCount++; // This field won't be updated in a template in an Android app
}

NgZone.run() workaround

This one is easier to use since you just need to wrap your code in it.

constructor(private zone: NgZone) {}

@HostListener("document:click")
onDocumentClick() {
  this.zone.run(() => this.documentClickCount++);
}

ChangeDetectorRef workaround

This one is faster than NgZone.run() since it triggers change detection only for a component that calls it.

However, you'll need to call ChangeDetectorRef.detectChanges() every time you return from your handler.

constructor(private changeDetectorRef: ChangeDetectorRef) {}

@HostListener("document:click")
onDocumentClick() {
  this.documentClickCount++;
  this.changeDetectorRef.detectChanges();
}

brian-weasner · 2023-03-06T17:46:14Z

I've run into this exact issue, and it has indirectly caused a hard to debug issue with firebase-js-sdk.

After updating to Firebase v9, we had to use the auth-compat library for some changes to slowly migrate, as we are using @angular/fire library in compatibility mode. iOS started breaking after the update, which shouldn't have happened.

After a bunch of debugging, it turns out the monkeypatch I added to fix issues with Angular location.back() not calling ngOnInit for routed components, caused issues with deviceready from immediately firing it's callback function if the listener was added after initial emit of deviceready.

Note: Loading the zone-patch-cordova.js fix did not resolve my issues with location.back().
- I belive this is due to zone.js being loaded after cordova.js due to where capacitor injects cordova.js.

The above monkeypatch is problematic with Capacitor, as capacitor loads native-bridge.ts and cordova.js in the top of the index.html head before my monkeypatch runs, as noted by the comments above.
The monkeypatch overwrites the logic in native-bridge.ts, which is required for document.addEventListener('deviceready', callbackFn) to immediately call it's callback function if the listener was added after initial emit of deviceready.

My Fix

I've updated my monkeypatch to include the same logic that is within Capacitor's native-bridge.ts, and moved my monkeypatch from my main.ts to the head in index.html

<script>
	window.addEventListener = function () {
		(window.EventTarget || Window).prototype.addEventListener.apply(this, arguments);
	};

	window.removeEventListener = function () {
		(window.EventTarget || Window).prototype.removeEventListener.apply(this, arguments);
	};

	document.addEventListener = function () {
		// Duplicating functionality from Capacitor.js native-bridge.ts, otherwise when cordova.js calls addEventListener for 'deviceready' will not immediately fire if 'deviceready' has already emitted.
		// Need to duplicate capacitor logic because capacitor injects native-bridge.ts code at the top of the head, so this function overwrites it.
		// @see https://github.com/ionic-team/capacitor/blob/89cddcd6497034146e0938ce8c264e22e7baba52/core/native-bridge.ts#L152
		const [eventName, handler] = arguments;
		if (eventName === 'deviceready' && handler) {
			Promise.resolve().then(handler);
		}
		// We don't care about back button error checks or default 'backbutton' action as we have @capacitor/app plugin installed, and set a listener within app.component.ts
		(window.EventTarget || Document).prototype.addEventListener.apply(this, arguments);
	};

	document.removeEventListener = function () {
		(window.EventTarget || Document).prototype.removeEventListener.apply(this, arguments);
	};
  </script>

AlanThiec changed the title ~~bug: Angular application + cordova.js file~~ bug: Angular application (Zone.js) + cordova.js file May 9, 2020

imhoffd added platform: android platform: ios good first issue bug labels Jun 23, 2020

KevinKelchen mentioned this issue Oct 16, 2021

RFC: Built-in Http module #5145

Closed

liamdebeasi mentioned this issue Jan 5, 2023

bug: @HostListener() on document events doesn't trigger change detection ionic-team/ionic-framework#26566

Closed

7 tasks

brian-weasner mentioned this issue Mar 6, 2023

Firebase web 9 Auth incompatible with Capacitor 3 firebase/firebase-js-sdk#5019

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bug: Angular application (Zone.js) + cordova.js file #2896

bug: Angular application (Zone.js) + cordova.js file #2896

AlanThiec commented May 9, 2020

AlanThiec commented May 28, 2020

imhoffd commented Jun 23, 2020

AlanThiec commented Jun 30, 2020 •

edited

Loading

KevinKelchen commented Feb 24, 2021 •

edited

Loading

Ventzy commented Oct 29, 2022

KevinKelchen commented Oct 29, 2022

matafokka commented Jan 6, 2023 •

edited

Loading

brian-weasner commented Mar 6, 2023

bug: Angular application (Zone.js) + cordova.js file #2896

bug: Angular application (Zone.js) + cordova.js file #2896

Comments

AlanThiec commented May 9, 2020

Bug Report

Capacitor Version

Affected Platform(s)

Current Behavior

Expected Behavior

Sample Code or Sample Application Repo

Reproduction Steps

Other Technical Details

Other Information

AlanThiec commented May 28, 2020

imhoffd commented Jun 23, 2020

AlanThiec commented Jun 30, 2020 • edited Loading

KevinKelchen commented Feb 24, 2021 • edited Loading

Ionic Angular with Cordova

Ionic Angular with Capacitor

Solutions?

Ventzy commented Oct 29, 2022

KevinKelchen commented Oct 29, 2022

matafokka commented Jan 6, 2023 • edited Loading

Problem

NgZone.run() workaround

ChangeDetectorRef workaround

brian-weasner commented Mar 6, 2023

My Fix

AlanThiec commented Jun 30, 2020 •

edited

Loading

KevinKelchen commented Feb 24, 2021 •

edited

Loading

matafokka commented Jan 6, 2023 •

edited

Loading