apache / cordova-lib

Apache Cordova Tooling Library
https://cordova.apache.org/
Apache License 2.0
221 stars 243 forks source link

Cordova doesn't work with NPM 7 #859

Closed rodrigograca31 closed 3 years ago

rodrigograca31 commented 4 years ago

Bug Report

I've been running the new npm version (7) and this is the first project that I had problems with.

Problem

Running the following commands gives an error:

cordova platform remove android
cordova platform add android --verbose

What is expected to happen?

Installation of cordova-android@^9.0.0

What does actually happen?

No scripts found for hook "before_platform_add".
No version supplied. Retrieving version from config.xml...
Grabbing pinned version.
Using cordova-fetch for cordova-android@^9.0.0
fetch: Installing cordova-android@^9.0.0 to /home/unknown/git/REDACTED_PROJECT_DIR
Running command: npm install cordova-android@^9.0.0 --save-dev
Command finished with error code 0: npm install,cordova-android@^9.0.0,--save-dev
Failed to fetch platform cordova-android@^9.0.0
Probably this is either a connection problem, or platform spec is incorrect.
Check your connection and platform name/version/URL.
CordovaError: CordovaError: Could not determine package name from output:
added 48 packages, and audited 1015 packages in 4s

4 packages are looking for funding
  run `npm fund` for details

14 vulnerabilities (12 low, 1 moderate, 1 high)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.
CordovaError: Failed to fetch platform cordova-android@^9.0.0
Probably this is either a connection problem, or platform spec is incorrect.
Check your connection and platform name/version/URL.
CordovaError: CordovaError: Could not determine package name from output:
added 48 packages, and audited 1015 packages in 4s

4 packages are looking for funding
  run `npm fund` for details

14 vulnerabilities (12 low, 1 moderate, 1 high)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.
    at /home/unknown/npm/lib/node_modules/cordova/node_modules/cordova-lib/src/cordova/platform/addHelper.js:288:31
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

Information

Like I stated before, its specific to npm 7, the new version that isn't out yet but its about to be https://github.blog/2020-10-13-presenting-v7-0-0-of-the-npm-cli/ https://blog.npmjs.org/post/626173315965468672/npm-v7-series-beta-release-and-semver-major

Command or Code

npm i -g npm@next-7 followed by cordova platform rm android && cordova platform add android --verbose the usage of npm 7 can be reverted with: npm i -g npm@6 which installs 6.x.x

Environment, Platform, Device

Environment:

    OS: LinuxMint 20 (linux 5.4.0-48-generic) x64
    Node: v12.19.0
    npm: 7.0.0

Version information

Cordova Packages:

    cli: 10.0.0
        common: 4.0.2
        create: 3.0.0
        lib: 10.0.0
            common: 4.0.2
            fetch: 3.0.0
            serve: 4.0.0

Checklist

breautek commented 4 years ago

Thanks for bringing attention to us.

This part of the codebase that handles fetching dependencies is actually part of cordova-lib so I'll be moving this ticket to that repo.

I've ran npm test on:

Only cordova-lib fails on NPM 7, so it looks like the might be the main package that requires updating for NPM 7 support.

breautek commented 4 years ago

I've also observed the same error on other commands that involves installing npm packages, such as cordova plugin add ...

dpogue commented 4 years ago

cordova-fetch is what deals with most of the calling out to npm, but more importantly also the parsing of the installation results. Might be worth trying to run the cordova-fetch unit tests with npm v7 and see what happens.

breautek commented 4 years ago

cordova-fetch is what deals with most of the calling out to npm, but more importantly also the parsing of the installation results. Might be worth trying to run the cordova-fetch unit tests with npm v7 and see what happens.

There are several failures in cordova-fetch as well on NPM 7, all in relation to unable to find package name from output.

dpogue commented 4 years ago

It appears that npm v7 doesn't print out a list of things that it installs (either as text or with the --json option, and not even with --verbose), which means we probably need to come up with a new strategy for dealing with installations 😞

As it currently stands, Cordova support for npm v7 is not going to be quick or easy.

raphinesse commented 4 years ago

Thanks for investigating @dpogue. It's really bad news for us if there's no way to get the installed package's name from the npm output. That might mean we have to change how we call cordova-fetch from lib. But I think lib does not know the package name in advance either. At least not in all cases (plugin add and platform add come to mind).

raphinesse commented 4 years ago

One approach would be to first resolve everything we need to manifests (which contain the package name) and tarballs, then install everything in one go. Our custom plugin dependency resolution will probably be a challenge, as usual.

Here's an example on how to get a manifest with npm's pacote. It also has builtin caching, so we should be able to pull this off without hitting the network multiple times for one package.

If done properly this has the potential to greatly improve our prepare speed in various scenarios, so there might come something good of it.

jordanaustin commented 3 years ago

I looks like there's an open PR to address support for npm@7 but no activity since October. Is this slated for an upcoming release?

breautek commented 3 years ago

I believe https://github.com/npm/cli/issues/2339 is the main blocker for https://github.com/apache/cordova-fetch/pull/91

raphinesse commented 3 years ago

Well, currently it looks as if npm/cli#2339 is seen as an intended change in behavior for npm v7.

I'm not entirely sure if this change would affect anything else than our current test-fixtures setup in cordova-lib. I would have to re-evaluate the situation given the latest comments from that issue to decide how to best address this. Unfortunately I'm a bit low on spare time right now.

However, the implementation of apache/cordova-fetch#91 would not change, regardless of npm/cli#2339. So we could push to integrate and release this (as a minor or fix release to cordova-fetch) and then it would be available for people. Then we could still figure out what to do about our tests in lib.

raphinesse commented 3 years ago

I just merged apache/cordova-fetch#91. If we release the current master of cordova-fetch as 3.0.1 then that should make cordova work with npm@7 without any other releases (users need to re-install cordova to get the fixed cordova-fetch).

However, I won't be able to do the release myself. So if someone could handle that task, that would be great.

raphinesse commented 3 years ago

A few days ago, we released cordova-fetch@3.0.1. This issue should now be resolved. Make sure to re-install cordova to get the latest version of that dependency.

yogskumar commented 3 years ago

Just use below command :

1) npm uninstall -g cordova 2) npm install -g cordova

Pradeepshivaram commented 1 year ago

This issue still seems to be present in the Cordova 12.0.0 (cordova-lib@12.0.1).

Cordova Packages

cli: 12.0.0
    common: 5.0.0
    create: 5.0.0
    lib: 12.0.1
        common: 5.0.0
        fetch: 4.0.0
        serve: 4.0.1

Environment:

OS: macOS Sonoma 14.0 (23A344) (darwin 23.0.0) x64
Node: v16.20.2
npm: 8.19.4

Any help to fix this would be appreciated!

breautek commented 1 year ago

This issue still seems to be present in the Cordova 12.0.0 (cordova-lib@12.0.1).

This a 3 year old issue was resolved and was related to changes introduced NPM 7 and was proven to be addressed in cordova-fetch@3.0.1.

If you're having an issue with the current CLI, then it will be best for you to create a new issue with updated/current information so that it can be tracked properly.