dpa99c / cordova-custom-config

Cordova/Phonegap plugin to update platform configuration files based on preferences and config-file data defined in config.xml
318 stars 84 forks source link

Including local plugin causes error #138

Closed jacobweber closed 6 years ago

jacobweber commented 6 years ago

I'm using cordova-custom-config 5.0.1, cordova-android 7.0.0, cordova-cli 8.0.0, and node 8.4.0, on Mac OS 10.13.2.

If my config.xml includes a plugin from a local directory, e.g.:

<plugin name="cordova-plugin-test" spec="custom_plugins/cordova-plugin-test" />

it causes the following error from cordova-custom-config, when I run cordova-prepare --verbose:

cordova-custom-config: Running applyCustomConfig.js
(node:70396) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): TypeError: Cannot read property 'getConfigXml' of undefined
(node:70396) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

This happens even if the custom plugin doesn't do anything.

Something that might be relevant: the latest version of cordova-cli seems to make node_modules/cordova-plugin-test a symbolic link to the original directory, instead of copying it, like cordova-cli 7.x did. I'm not seeing this error with cordova-cli 7.x.

A sample project is attached; just install cordova-cli 8.0.0 and run cordova-prepare --verbose. CordovaTest.zip

dpa99c commented 6 years ago

This seems to be a bug in the cordova@8.0.0 CLI: for some reason, inclusion of the local plugin causes installation of this plugin's node dependencies to fail. If you look at the contents of the project's node_modules/ directory, you'll see it's virtually empty whereas it should contain the node modules listed as dependencies by this plugin.

The error message is a bit confusing due to some dodgy error handling by this plugin that can probably be improved: this is what's currently happening:

The error handling of this plugin could certainly be improved (I'll have a go at that and publish as @5.0.2) but the root cause of the failure is cordova@8.0.0 failing to install the dependencies of this plugin when a local plugin has been referenced before it in the config.xml.

We should open an issue against cordova@8.0.0 using your test project as an illustration of the bug.

dpa99c commented 6 years ago

On further inspection of your test project, it seems the problem is that it was missing a package.json file and this was causing Cordova to omit installation of the plugin dependencies. Once the auto-generated package.json is in place, the cordova prepare operation is successful.

I've updated your test project to add the package.json - you can try it yourself: CordovaTest.zip

For this reason, I'm not going to raise a bug against cordova@8.0.0 using this test case, because I'm pretty sure they'll say the package.json is a required file and that its absence is an error in the project structure rather than the Cordova CLI. However, if you think this should not be the case, you are welcome to open an issue against it yourself.

I'm closing this issue on the basis that the resulting error is due to the lack of a package.json file and not a bug with this plugin.

FYI I've published @5.0.2 of this plugin which better handles errors when loading dependent modules.

jacobweber commented 6 years ago

Hi @dpa99c — thanks for taking a look at this. Hope I didn't waste too much of your time on it. I had always assumed that I could let the "cordova prepare" command create package.json. Maybe I'll report this to them anyway, and see what they say.