mapiacompany / capacitor-codepush

Capacitor plugin for CodePush
http://appcenter.ms
Other
151 stars 65 forks source link

TypeError: undefined is not an object (evaluating 'window.codePush.reportStatus') #31

Open lincolnthree opened 3 years ago

lincolnthree commented 3 years ago

Thanks so much for filing an issue or feature request! Please fill out the following (wherever relevant):

Description

I'm not sure why this is happening, but a small subset of devices the codePush instance seems unavailable in the window context. Possibly a timing issue? This is strange though because our app waits a solid 4-5s before attempting any CodePush actions.

TypeError: undefined is not an object (evaluating 'window.codePush.reportStatus')
  at ionic:(//localhost:1:71)
  at ? (ionic://localhost/polyfills-es2015.6efff9e9e083b048c166.js:1:40938)
  at runTask(ionic://localhost/polyfills-es2015.6efff9e9e083b048c166.js:1:11465)
  at invokeTask(ionic://localhost/polyfills-es2015.6efff9e9e083b048c166.js:1:17103)

Reproduction

No reproduction available currently. Just production logging messages.

Additional Information

(The more info the faster we will be able to address it!)

lincolnthree commented 3 years ago

Actually this seems to be quite common:

image

lincolnthree commented 3 years ago

This appears to be an issue when the CodePush TS/JS library is not loaded at the time when the native plugin attempts to register the onDeviceReady listener. The injected/eval JS hooks in the native CodePush manager code for Android/iOS should probably be refactored to handle this situation.

Since Capacitor does not guarantee the client JS code is loaded until the application imports the CodePush libraries (unlike Cordova which loads all plugin code up front), we will need to provide deferred callback support in the platform reporting managers:

  1. https://github.com/mapiacompany/capacitor-codepush/blob/main/ios/Plugin/CodePushReportingManager.m#L36
  2. https://github.com/mapiacompany/capacitor-codepush/blob/main/android/src/main/java/com/microsoft/capacitor/CodePushReportingManager.java#L32

Something like this (I'm not sure of the syntax needed, I think this is wrong but the intent is right.):

if(window.codePush) {
   javascript:document.addEventListener(\"deviceready\", function () { window.codePush.reportStatus(%d, %s, %s, %s, %s, %s); });
} else {
   window.codePushReportStatus = function () { window.codePush.reportStatus(%d, %s, %s, %s, %s, %s); });
}

Then in codePush.ts here: https://github.com/mapiacompany/capacitor-codepush/blob/main/src/codePush.ts#L525


export const codePush = new CodePush();
(window as any).codePush = codePush;
if(window.codePushReportStatus) {
   window.codePushReportStatus();
   window.codePushReportStatus = undefined;
}
```f
lincolnthree commented 3 years ago

@leo6104 What do you think?

lincolnthree commented 3 years ago

As a workaround. CodePush needs to be loaded immediately on App start. (I've done this in the Angular root app TypeScript file.) But this is not ideal since it increases the initial JS bundle size and app load times. (Bad for PWAs & user experience.)

import { codePush } from 'capacitor-codepush';
(() => codePush)(); // to prevent it from being minified away
o-alexandrov commented 3 years ago

@lincolnthree sorry for pinging about a different issue, but it seems your input might be valuable in #21

Please let us know whether you experience such issue on iOS. It would let us know there whether issue is related to our own setups, or it's not working for everyone.

lincolnthree commented 3 years ago

@o-alexandrov It does actually happen on iOS. I've amended my issue report.

lincolnthree commented 3 years ago

@o-alexandrov BTW. It's probably better to actually ping me in the issue you'd like me to look at, rather than convolute the discussion of another topic. The topic you're interested in is unrelated other than the platform is may occur on, which I'm not sure is a really strong 'link'. Lots of platform specific bugs occur in hybrid codebases due to the double coding required to support each platform.