CB-12242 : Use yarn instead of npm

[qkdreyer]

Platforms affected

All

What does this PR do?

Allows user to select his package manager used by cordova-fetch, possible values are "npm" or "yarn", defaulting to "npm"

What testing has been done on this change?

None, since we're just adding cordova arguments here

Checklist

• Reported an issue in the JIRA database
• Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
• Added automated test coverage as appropriate for this change.

[janpio]

Tests are failing because of a code style problem:

C:\projects\cordova-cli\src\cli.js
496:34 error Unexpected trailing comma comma-dangle

Could you please fix that so the actual tests can run an this can possibly be merged? Thanks.

[HeatherLemieux]

Any updates on this? Would love for this to get merged in.

Thanks!

[qkdreyer]

I've just fixed the code style issue. Could you merge it @janpio ?

[janpio]

Nope sorry, I don't know anything about the cordova-cli codebase. Hope someone else who does will come along and review and merge this.

[japj]

Does this also depend on apache/cordova-fetch#14 ?

[qkdreyer]

It depends on both apache/cordova-fetch#14 and apache/cordova-lib#610

[japj]

is there anyone that can help with progressing this item further?

[Bessonov]

why not just args.manager || 'npm'?

[c-emil]

Can anyone move forward with this PR please? It would be really appreciated to have this merged.

[dpogue]

Not speaking for anyone else here, but I'm not especially in favour of adding additional complexity and maintenance burden to several layers of Cordova libraries for very little benefit. Cordova made the decision to use npm, and with the improvements in npm 5 and 6 it's arguable whether there's any advantage to yarn (especially in this limited use case).

We know that there are some issues with how cordova-fetch is using npm, but I'd rather fix those as opposed to doubling the amount of testing everyone needs to do to support two package managers with different command syntaxes.

[japj]

I'm not familiar with any recent changes to npm w.r.t. offline support, but we have internally standardized on yarn for offline mirror support during building (also for not-cordova related projects). I can imagine that might also be the case for other people/companies? I'd be interested in thoughts about resolving that in a nice way

[Bessonov]

Fully agree with @dpogue. Personally I think it's OK to provide some parameter to call external managers or scripts to do tasks, but without any support. This pull request do it wrong.

[qkdreyer]

Not sure what do you mean here guys...

[raphinesse]

@qkdreyer I can understand your desire to use your package manager of choice with cordova. But with how Cordova currently uses npm, adding support for additional package managers is just not feasible.

I just recently completely refactored cordova-fetch, the main component to use npm in Cordova. This resolved a massive amount of bugs and performance problems. Unfortunately, the new implementation had to be even more tightly coupled to npm to achieve this. However, the performance problems you mentioned in the original issue might be resolved already in cordova@nightly because of this. Please give it a try.

My long term goal is to get to a point where Cordova does not have to do any package management at all. But that's still a long way to go, if we will ever get there. Until then, I would like to avoid any additional complexity introduced to the whole package management process.

I'll play the bad guy here and close this and the other related PRs. I'm under the impression that this reflects the general stance among the Cordova tooling team. Please note that my decision is not authoritative in any respect. If someone disagrees, they should just reopen.

I'm sorry that this did not come up during discussion in the issue before you created your PRs. 🙇‍♂️

[jayvdb]

fwiw, I was able to get yarn working with cordova@8 and a very basic yarn wrapper (below) and a symlink npm. The wrapper is needed because npm install foo needs to invoke yarn add foo.

#!/bin/bash

args="$@"
arg2="$2"
if [ "$1" = "install" ] && [ -n "$arg2" ] && [ "${arg2/--//}" = "$arg2" ] ; then
    args="${args/install/add}"
fi

yarn $args

A better version of the wrapper is needed to get --save-exact working.

And pnpm with a symlink npm works without any wrapper.

No doubt this wouldnt cover all possible usages, and shouldnt be supportable by the core team, but it seems reasonable that cordova-fetch can use other package managers used by the project (e.g. detected via apache/cordova-fetch#46 , or an explicit and non-friendly option ), and include a warning that only npm is supported.

[janpio]

(Best create a new issue so this doesn't get lost @jayvdb as a possible starting point)

[jayvdb]

I think apache/cordova-fetch#46 is a reasonable starting point, as the work would need to be done in cordova-fetch first, and that is where the complexity of tight integration with npm exists.

[marckassay]

@jayvdb, inspired by your workaround I developed a package (spypkg) to cover most possible usages. When installed, you should be able to use cordova with yarn. Tested on Ubuntu and Windows.

[adelmojunnior]

Hi @marckassay!
I'll create a ionic 4 project blank and add spies in windows, but when execute ionic cordova platform add android, the package.json.lock is created. How resolve this to dont create the lock file?

[abdrahim-au]

Hi @jayvdb
your shell is helpful at now, thanks
i got a problem when i tried to add plugin, yarn say error Command "view" not found so also npm view foo needs to invoke yarn info foo
i don't know much about shell but makes a try and its work fine :)

#!/bin/bash

args="$@"
arg2="$2"
if [ "$1" = "install" ] && [ -n "$arg2" ] && [ "${arg2/--//}" = "$arg2" ] ; then
    args="${args/install/add}"
fi

if [ "$1" = "view" ] && [ -n "$arg2" ] && [ "${arg2/--//}" = "$arg2" ] ; then
    args="${args/view/info}"
fi

yarn $args