Unified cordova fetching and create cordova-fetch module

[stevengill]

Take a look at the proposal and let me know what you think.

[dblotsky]

Will this work with NPM 3+ and previous versions?

[stevengill]

It should work with npm 3 and npm 2. It would use whatever the user has installed.

[nikhilkh]

There is an advantage to locking down version of npm as we currently do. As on different machines different versions of npm might not be compatible even for the same version of cordova. We might need to warn if that happens instead of it failing with some random error on certain machines.

[nikhilkh]

Overall, this looks like a good refactoring to pursue - however, we might need a lot of tests to ensure we do no have any regressions. :)

[stevengill]

Thanks @nikhilkh!

Definitely going to need to test this well to make sure we don't have regressions. It will be important to test this with at least all major versions of npm (1, 2, 3).

I do see the advantage of locking down the npm version (no surprises with system versions). But I don't foresee any issues with using the system npm. We would just be using npm cache add and then following it up with a npm install.

Running npm install using npm programmatically in a specific directory was proving very difficult. I tried many different ways to change the working directory to ~/.cordova/lib/npm_cache/PACKAGE to run npm install. I eventually came to the conclusion that shelling out was needed to do this.

It is actually the recommended to shell out to system npm over using require('npm').

Sidenote: We also have to take into account some of the npm utility methods we created. [1]. Will have to see if it makes sense for this to live in cordova-fetch.

[1] https://github.com/apache/cordova-lib/blob/ac57bebf3b567f825fe6e586b34ddaa748a27185/cordova-lib/src/cordova/util.js#L352

This obviously won't be ready for cordova 6 release.

[csantanapr]

@stevengill this looks good same thing we talked on Slack.

Remind me again why do we need to run 'npm cache add' ?
I would stay away from cache, I would consider it a private npm api

why no just rely on 'npm install' and let npm decide if it will use the cache or not, or check if its in the cache or not and son on...

[stevengill]

To be honest, I don't have a good reason to stick to npm cache add over plain old npm install. It was mainly to not change things too much from how they are now.

My short term plan was to continue using npm cache add (although moving things over to ~/.npm instead of ~/.cordova/lib/npm_cache) and then run npm install on the repo. Then at a later date, I was going to propose adding package.json during cordova create and cordova platform add android --save would run npm install cordova-android --save in the cordova project. So cordova projects would have a node_modules directory where the platforms and plugins would live.

Going the package.json route is going to require more work and more discussion to make sure everyone is on board. I still plan to pursue this.

I think it makes sense to use npm install instead of npm cache add. Any suggestions on where I should store these modules? Maybe ~/.cordova/lib/modules? npm install actually runs npm cache add so the modules will still be added to ~/.npm. I believe npm install also checks the cache to see if it needs to hit the registry or can just use the cached version.

[stevengill]

@nikhilkh what are your thoughts on the above?

[csantanapr]

@stevengill OK make sense this is just a baby step in the final goal of using package.json and project/node_modules

Let's skip using putting our hand into ~/.npm or using npm cache add.

Sounds good to use ~/.cordova but I would use ~/.cordova/lib/node_modules instead

This allows if the user wants to pre-load the library it should only need to run npm install in the directory ~/.cordova/lib/ (copy the node_modules from another place)

This allows to build servers for ci/builds that don't have access to npm or network, he can load the library with [email protected], [email protected], [email protected], , and even private plugins.

Then using cordova 6 and adding specific versions of platforms and plugins cordova-cli will just grab them from the library (~/.cordova/lib/node_modules) if present then avoiding calling npm or connection to network.

thoughts?

[dpogue]

These have been discussed in JIRA and Slack, but just making a note here to capture them:

• CB-10438 The dependencies specified in plugin.xml should respect the version attribute when fetching other plugins.
• CB-10239 It should be possible to install platforms that cordova doesn't know about.
  This is a more general case to support installing platforms from scoped npm packages, but a blocker there is that cordova tries to validate the platform name before fetching it.

[stevengill]

Thanks @dpogue!

I have a board to track all of these issues and more. https://issues.apache.org/jira/secure/RapidBoard.jspa?rapidView=109&view=detail

[stevengill]

So I need to revise this proposal after running into some issues. Before I get to the issues, I made some modifications of how cordova-fetch proposal I was working on.

Mainly, both gitURLs and PackageIDs get npm installed. npm install can support all of the same gitURLS we take except subdirectories cordova plugin add https://github.com/someone/aplugin.git#:/my/sub/dir. I was thinking we continue to handle subdirectories in cordova-lib. (Strip it from the url before passing to fetch, manually cd into subdirectory on return)

This idea of npm install all the things would remove our need to shell out to git. It would also eventually make saving very easy as we would just pass the --save option right to npm install and it would save package.IDs and gitURLS right to package.json. Of course that would only be beneficial when we actually add it finally.

Local paths never need to get fetched so they never make it to cordova-fetch.

Now, my two separate issues here.
1.) npm@3 flattening issue. Example:
cordova plugin add cordova-plugin-device, this npm installs the plugin to ./cordova/lib/node_modules. Lets say the device plugin has a dependency on q. So q is now a sibling of device in ./cordova/lib/node_modules. cordova-fetch passed the path to device plugin back to cordova-lib. cordova-lib now copied the device plugin into MYAPP/plugins. It doesn't know that q is a sibling and it doesn't get copied.

2. not knowing what was installed issue. Example:
   Install via gitURL where the repoName != package.ID
cordova plugin add https://github.com/stevengill/npmInstallTest.git. Package.id === not-repo-name. This will install to ./cordova/lib/node_modules/not-repo-name. npm install doesn't let me know what was just installed. so I have no way of getting a reference to that directory and returning it in cordova-fetch.

Potential solutions:
For 1) I'm thinking we stop trying to npm install to this middle directory of ./cordova/lib/node_modules and go directly to myApp/node_modules. No need to do a copy. Any scripts that require('q') can reference it because it exists in node_modules.

For 2) I'm thinking of using a diff of the dependency tree when installing a gitURL to see what was installed and return that. Alternatively, I could not use npm install for this and continue to rely on our git clone to a tmp directory code. I'm exploring both options.

So this proposal will have to be done in conjunction with cordova-create and adding package.json.

It is more work than I was hoping to do at this time but I think it is the best long term solution. Definitely cordova@7

Thoughts?

[dpogue]

As expected, I'm totally 💯 👍 on the node_modules idea, and saving to package.json.

[nikhilkh]

I would recommend approaching this work incrementally as you had originally planned. Ideally, we can keep all the current code working as is and have an opt-in mechanism for the new path using npm-fetch and get feedback by releasing with it every release of 6.x. We know this will have a significant impact on downstream tools, distributions, and build servers.

Moving from storing plugins/platforms from config.xml to package.json exposes this to the users as well - and would be disruptive for upgrade scenarios. It's hard to foresee how this play out and using an incremental approach with every release would be great.

Perhaps this is great topic for a google hangout discussion. We haven't had one in a bit. :)

[stevengill]

Opened a PR with fetch. So far, I've added only integrated it with platform add/rm/update and behind the --fetch flag. cordova-lib: apache/cordova-lib#407, cordova-cli: apache/cordova-cli#239

A few things to note:

• we will have to drop support for installing subdirectories via git. https://cordova.apache.org/docs/en/latest/cordova-cli/index.html#plugin-spec. npm install doesn't support subdirectories. If this is functionality we want, we should work with npm to get this added to their cli. We should add a deprecation notice for this soon in cordova 6. If we were to eventually move saving into package.json, having a gitURL with our subdirectory syntax would fail during a npm install command.
• cordova platform update command wasn't generating a npm ls diff in my node_modules directory due to us not removing and re-adding the platform. npm install cordova-android@latest would update the version of cordova-android in node-modules, but my technqiue for getting what module was installed wouldn't catch those changes. I have added a backup method to determine the ID of the fetched module but it won't be able to handle the usecase when installing from git and reponame != packageName. Changing update to be a alias for platform add and then platform rm would remove the need for this backup method.
• cordova platform rm android deletes the cordova-android directory from node_modules. A better solution would be to create a cordova-remove module that can be shared among platform and plugin remove. It would run npm uninstall --save in your cordova project. A future incremental step.
• no package.json yet. Going to do it incrementally as @nikhilkh suggested.
• cordova platform add is creating a node_modules directory in your cordova project and npm installing the platform into it.

Plugin and template integration to come shortly.

[stevengill]

Also, still need to integrate apache/cordova-lib#382 so we can get logs/verbose events passed into cordova-cli.

[stevengill]

Another point, every module needs a package.json now. So plugins without package.json need to add one to work with fetch

[stevengill]

I updated the proposal yesterday. Let me know your thoughts @nikhilkh @purplecabbage @dpogue

[nikhilkh]

I'm assuming if --fetch is not specified we should not save it to package.json.

[stevengill]

yup

[dpogue]

I'm all in favour of storing platform and plugins in package.json, but I don't think all of config.xml should be moved there. The rest of config.xml (the actual application config stuff) should aim to move to the W3C Web App Manifest format.

[nikhilkh]

While I like that we do not have to support git cloning in our own code. There are some advantages that this code provides us - and one of them is to be able to control some of these features. There is also some desire to support submodules:
apache/cordova-lib#424
#38

Letting npm do it's thing - implies we might have to wait for npm to support these features.

[stevengill]

If the goal is to move plugins and platforms to package.json under dependencies, then we have to align with what npm supports. I think this should be the goal.

If we keep our own git cloning around (that supports subdirectories and submodules), we would be forced to continue to save our dependencies in config.xml or some other non standard attribute in package.json. Saving git references npm doesn't support under package.json dependencies attribute is asking for trouble.

I think our code would be simpler to maintain, and app dependency management would easier to understand for cordova app developers if we adopt the standard npm way of doing things. We can then maybe focus on contributing support for subdirectories/submodules to npm itself.

[csantanapr]

I think we try to support most common use cases, and most success for users.

Supporting subdirectories and submodules, its an advance/edge case that can be manage by the user to do the git cloning downloading and use the local folder , npm also support specifying dependecies using file:../foo/bar

meaning you can npm install /dir/mygitproject/somesubdir/withnpmpackage.json or cordova plugin add /dir/mygitproject/somesubdir/ --fetch

[stevengill]

@dpogue We can discuss that more when the time comes. I'm also not entirely sure it makes sense to move everything over from config.xml to package.json but once the time comes, we can make a list and see what makes sense.

At the very least, engine tags & platform/plugin save should be moved over to use package.json.

[csantanapr]

I agree time will come to create a table what stays in config.xml and what goes into package.json.

At least in my mind I see it as a separation of concerns where:
config.xml : contains information when the app runs on the device (runtime configuration)
package.json: contains information about app tooling, source code, build, dependency components (i.e. app project config)

[brody4hire]

There is also some desire to support submodules:
apache/cordova-lib#424
#38

To be honest I would favor letting npm fetch the dependencies or subprojects as well. I found git submodules to be a pain to deal with. For example, building SQLCipher for Android: https://github.com/sqlcipher/android-database-sqlcipher

[brody4hire]

While I like that we do not have to support git cloning in our own code. There are some advantages that this code provides us - and one of them is to be able to control some of these features.

Could we add a "git" flag to fetch using git, which would allow subdirectories, and use npm otherwise?

[stevengill]

@brodybits the issue is where to save the dependency if we use the --git flag. My ultimate goal here is to save dependencies in package.json. You can't/shouldn't add a git reference that npm can't handle in the dependency tag in package.json.

[purplecabbage]

github is supported via an id of 'gitusername/gitreponame'

Is there a reason to support it more deeply than that?

@purplecabbage
risingj.com

On Fri, Apr 22, 2016 at 11:34 AM, Steve Gill [email protected]
wrote:

@brodybits https://github.com/brodybits the issue is where to save the
dependency if we use the --git flag. My ultimate goal here is to save
dependencies in package.json. You can't/shouldn't add a git reference that
npm can't handle in the dependency tag in package.json.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#33 (comment)

[stevengill]

Github will be supported as npm supports it. That is what I am going for.

What we won't be able to support are subdirectories (one repo with many plugins) and git submodules as npm doesn't support them.

[csantanapr]

I agree with Steve, we should draw a line we can't support everything.
There are many ways users can workaround it's up to them. At least one that I prefer is using branches to distinguish the assets to deploy like "release" branch you deploy from there, and the "master" or "develop" branch where dev happens.
More info on Git URLs as Dependencies here:
https://docs.npmjs.com/files/package.json#git-urls-as-dependencies

git://github.com/user/project.git#commit-ish
git+ssh://user@hostname:project.git#commit-ish
git+ssh://user@hostname/project.git#commit-ish
git+http://user@hostname/project/blah.git#commit-ish
git+https://user@hostname/project/blah.git#commit-ish

[nikhilkh]

Here's the scenario:
I am setting up a new machine and just downloaded a cordova project from the source control. What am I supposed to do in the "fetch" world?
"npm install" or "cordova prepare". If I do npm install - then how does the plugins/ directory get hydrated? If I do cordova prepare - do I have to do npm install later as well? Sounds like the ordering of these operations is important. Am I missing something here?

[csantanapr]

I really don't like that "cordova prepare" install dependencies
I think for for CLI 7.x we stop this, and have the user "npm install" or come up with a "cordova install" that parse config.xml and maybe also "npm install"

"cordova prepare" should only be concern with small task it got too broad of a kitchen sync

[nikhilkh]

Who creates the platforms/ & plugins/ directory? Someone has to...

[csantanapr]

cordova platform add and cordova plugin add takes care of creating those directories.
In addition like I said running "cordova install" it will run "platform add" and "plugin install" to move content/copy/install content from node_modules/ to platforms/plugins/

[stevengill]

cordova prepare currently restores based on what is saved in config.xml and what is installed. I would think it would continue to do the restore step but use package.json instead of config.xml.

So if a user runs npm install, then cordova prepare, it would see that those plugins & platforms aren't "installed" according to cordova, so it would go through the motions of fetching (Which is already done since the user ran npm install), then copying to platforms/plugins.

If a user doesn't run npm install, cordova prepare would detect that those plugins/platforms aren't "installed" and move forward with fetching.

So cordova can support both usecases 😄

[nikhilkh]

Thanks, Steve! It will be great to add the details of how that will work to this proposal. However, diffing does not work in this case - correct? Are you thinking there would be custom json properties in package.json that identify plugins and platforms in addition to them being referenced in "dependencies"?

[csantanapr]

I remember problems reported by community members maybe @dpogue that installing things with "cordova prepare" was too much magic and not handling well some use cases, and a "cordova install" would make more sense, or something explicit because right now is preparing and installing if missing, so something like "cordova prepare --install" it will prepare and install just in case.

Maybe @dpogue can provide more details about having a "cordova install"

[dpogue]

The only problem I recall with cordova prepare is that it makes testing and building a single platform difficult in a multi-platform project.
If my config.xml has both iOS and Android platforms, and I run cordova platform add ios && cordova build, it will trigger a prepare that will also install the Android platform.

I think the best option would be for cordova prepare to copy the platforms and plugins from node_modules as needed, but rely on npm install for installation and fetching.

[vladimir-kotikov]

+1 to @nikhilkh's concern about detecting already fetched/installed plugins. I'd suggest to add a property to every package.json to indicate the type of package: plugin or platform. Also having such we could easily enumerate installed plugins just by checking every module's package.json (and probably eliminate the need for copying these plugins from node_modules to plugins directory)

[stevengill]

Not a bad idea @vladimir-kotikov. Worth revisiting once first implementation is complete.

[riknoll]

If we support installing plugins using npm install rather than doing all cordova-related things though our CLI, then do we lose some control over the installation process? For example, the plugin version selection stuff released in 6.1.0 wouldn't get applied in that case. Running npm install example-plugin and cordova plugin add example-plugin could return different things.

[vladimir-kotikov]

Another idea (crazy enough, but it let us to transfer even more responsibilities to npm) is that we might just run npm uninstall as a main action and do the rest of the things to actually remove plugin/platform via NPM hooks.

[stevengill]

I've thought about this too. It is too big of a change IMO. Every third party plugin would need to add that hook.

[vladimir-kotikov]

Hook is not necessarily needs to be shipped with dependency package - we can put our own hook into node_modules/.hooks/uninstall and it'll be called for every package being removed.

Actually, at the second thought it might be not a good idea, as it would spread uninstallation logic onto two separate places and would make maintaining it too complex ;(

[nikhilkh]

It will be great to highlight what are the key merits of this new approach to an end user - my concern with using npm's persistence mechanism - package.json and npm commands like "npm install" - we lose control over the workflow - where & what files get downloaded, how constraints to other components are matched etc. - as we try to address some of those limitations, we might create more complexity and dependencies on how npm currently behaves (node_modules directory structure).

Having our own commands - cordova plugin add, and our own persistence format - config.xml affords the flexibility and control of doing what's right for cordova customers which are dealing with native SDKs of multiple platforms. Our plugins and platforms not a "typical" npm module (they ship java, objective-c code - not only js) and I wonder if we should even aspire to be one. Config.xml is not going away anytime soon - there is a lot that needs to be encoded there e.g. the current PR to manipulate manifests.

[brody4hire]

I also don't like losing "control over the workflow". npm is great for what it does but was never designed for handling multiple native platforms and cannot even handle native code dependencies between modules (unless I am mistaken).

Extra rant: The one thing I don't like is that config.xml is a bit too generic in case a project is designed to work with multiple frameworks and another framework also uses config.xml.

[stevengill]

I think this discussion in here has gone out of scope for this proposal.

Lets save npm install vs cordova platform/plugin add/rm for a later time. Same goes for package.json vs config.xml.

[stevengill]

I updated the proposal. --template now supports --fetch. It fetches it to ~/.cordova/node_modules and copies of the necessary files.

I believe this proposal can be merged now.

[nikhilkh]

LGTM. Lets merge this.

[csantanapr]

LGTM