OneSignal / OneSignal-Cordova-SDK

OneSignal is a free push notification service for mobile apps. This plugin makes it easy to integrate your Ionic, PhoneGap CLI, PhoneGap Build, Cordova, or Sencha Touch app with OneSignal. Supports Android, iOS, and Amazon's Fire OS platforms. https://onesignal.com
Other
251 stars 198 forks source link

crash during compile 3.1.0 #805

Closed marsiliflavio closed 2 years ago

marsiliflavio commented 2 years ago

Hi, during compile .aab latest ver 3.1.0 crash. If use 3.0.4 all ok. ANDROID_PLATFORM_VERSION=10.1.2 cordova@11.0.0 Gradle 7.1.1 command failed npm ERR! command sh -c yarn run build npm ERR! sh: 1: yarn: not found

thanks

nan-li commented 2 years ago

Hi @marsiliflavio, Thank you for reporting this. I'm not sure but I do see yarn run build in your error and it seems that you don't have yarn installed. For context, in release 3.1.0, we converted our codebase to TypeScript and there is a script to run yarn run build that transpiles the SDK to Javascript, and this runs before we publish this plugin to npm.

But this should not be affecting users, as yarn run build should not be run except when we publish to npm. Can you share how you are installing onesignal-cordova-plugin? And how you are compiling the .aab?

marsiliflavio commented 2 years ago

Hi, thanks for the support. I use https://monaca.io/ in cloud for deployment. I don't know exactly how their compiler is doing.

jkasten2 commented 2 years ago

@marsiliflavio Thanks for the details, we will look into addressing this. It's odd this is creating an issue and can't reproduce with local builds (even when yarn is not installed on the system). We are looking into changing our SDK to use prepublish instead of prepare to attempt to address this specific issue with monaca.io.

emccorson commented 2 years ago

I encountered the same problem specifically when onesignal-cordova-plugin is installed as git dependency:

"dependencies": {
  "onesignal-cordova-plugin": "git+https://github.com/OneSignal/OneSignal-Cordova-SDK"
},

From the npm docs, it appears that installing from git will run the prepare script:

When installing from a git repository, the presence of certain fields in the package.json will cause npm to believe it needs to perform a build. To do so your repository will be cloned into a temporary directory, all of its deps installed, relevant scripts run, and the resulting directory packed and installed.

This flow will occur if your git dependency uses workspaces, or if any of the following scripts are present:

build prepare prepack preinstall install postinstall

I tested this on Monaca and specifying the package version instead of the git URL fixed the problem:

"dependencies": {
  "onesignal-cordova-plugin": "3.1.0"
},

That might be the same thing that is going on here.

marsiliflavio commented 2 years ago

Great! Great! here is my proof:

Thanks!!!!...and CIAO

nan-li commented 2 years ago

Thanks @marsiliflavio and @emccorson for digging into this.

Ahh this makes sense. I had before wrongly assumed how you installed the plugin.

If you are installing directly from the git, the plugin will be installed locally, and the prepare script needs to run in order to transpile the TypeScript code into JavaScript. What happens is a new dist directory is created with the JavaScript code, and this is what users of the plugin actually use. Our GitHub does not contain this generated directory so the script needs to run to generate it.

However, if you download with something like npm install onesignal-cordova-plugin or use dependency like "onesignal-cordova-plugin": "3.1.0", it is using the remote package that already has the dist folder, so no scripts need to run.

We will look into how to make this difference clearer for users.

Note: Able to reproduce this error by uninstalling yarn and installing onesignal-cordova-plugin by npm install https://github.com/OneSignal/OneSignal-Cordova-SDK.