Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cordova prepare not resolving platform specification correctly for devDependency #832

Closed
3 tasks done
grunthor opened this issue May 8, 2020 · 1 comment · Fixed by #835
Closed
3 tasks done
Assignees
Milestone

Comments

@grunthor
Copy link

grunthor commented May 8, 2020

Bug Report

Problem

when running cordova prepare for a project with a platform as a devDependency in the package.json, the version is not discovered correctly.

The manual moving of the platform from dependencies to devDependencies is suggested by the verbose logging of cordova-fetch and issues apache/cordova-fetch#64 and apache/cordova-electron#88.

And the changes from pull request apache/cordova-fetch#65 will result in the platforms being added to devDependencies by default.

What is expected to #happen?

With

"dependencies": {
},
"devDependencies": {
    "cordova-electron": "^1.1.1"
},

running cordova prepare after removing platforms, should fetch cordova-electron ^1.1.1
expected console log (this occurs if the platform is in dependencies):

Discovered platform "electron@^1.1.1" in config.xml or package.json. Adding it to the project
Using cordova-fetch for cordova-electron@^1.1.1

What does actually happen?

instead, this tries to fetch cordova-electron ^1.0.0
actual console log:

Discovered platform "electron" in config.xml or package.json. Adding it to the project
Using cordova-fetch for cordova-electron@^1.0.0

Information

This seems to be a result of an if else construct the addHelper.js
https://github.com/apache/cordova-lib/blob/master/src/cordova/platform/addHelper.js#L96-L108
It looks like this finds an empty dependencies object, and then does not subsequently check the devDependencies object contents at all.

Command or Code

reproduction method: (courtesy of @erisu )

cordova create electronTest
cd electronTest/
$ cordova platform add electron
Using cordova-fetch for cordova-electron@^1.0.0

$ cordova prepare
[Cordova Electron] The built package size may be larger than necessary. Please run with --verbose for more details.

rm -rf platforms

$ cordova prepare
Discovered platform "electron@^1.1.1" in config.xml or package.json. Adding it to the project
Using cordova-fetch for cordova-electron@^1.1.1

rm -rf platforms

vi package.json
/// HERE I am moving to devDependencies

$ cordova prepare
Discovered platform "electron" in config.xml or package.json. Adding it to the project
Using cordova-fetch for cordova-electron@^1.0.0

Environment, Platform, Device

Running on Windows 7

Version information

Cordova 9.0.0 ([email protected])

Checklist

  • I searched for existing GitHub issues
  • I updated all Cordova tooling to most recent version
  • I included all the necessary information above
@erisu erisu self-assigned this May 8, 2020
@erisu erisu added this to the 10.0.0 milestone May 8, 2020
dpogue added a commit to dpogue/cordova-lib that referenced this issue Jun 2, 2020
This also includes fixes for writing package.json with the correct
newlines, which should address the complaints raised in [1] and possibly
[2].

1. apache/cordova-cli#353
2. apache#694
dpogue added a commit to dpogue/cordova-lib that referenced this issue Jun 2, 2020
This also includes fixes for writing package.json with the correct
newlines, which should address the complaints raised in [1] and possibly
[2].

1. apache/cordova-cli#353
2. apache#694
@erisu erisu closed this as completed in #835 Jun 3, 2020
erisu pushed a commit that referenced this issue Jun 3, 2020
* GH-832: Look at devDeps for restoring platforms

This also includes fixes for writing package.json with the correct
newlines, which should address the complaints raised in [1] and possibly
[2].

1. apache/cordova-cli#353
2. #694

* Ensure a failure to restore stops the build

This has been a recurring frustration over several years, and nobody can
seem to explain why we would want to silently ignore restore failures
for platforms and plugins rather than surfacing them and failing the
remainder of the build steps.
@Crono1981
Copy link

I think this should be reopened. The same thing happens with [email protected]. The prepare command will go and fetch [email protected].

As a workaround I've added the platform as a runtime dependency rather than a development dependency (using the --save option with cordova platform add).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants