Improve dependency inclusion

[SrTobi]

The current ignore algorithm for devDependencies is pretty simple and does not really work for vscode extensions. That's mainly because productive dependencies of devDependencies are not ignored and vscode has a lot of them. I tried to fix this by using npm itself: using the ls command npm builds a dependency structure that can be used to extract all real productive dependencies. In a second step all files in the selected modules are globed and put into a list.

It works quite well and has the advantage that also modules that are in other modules treated in a correct way. There are still a few issues though:

• The main problem is that until now I have not found a way to silent the npm output
• Because of npm the nodejs tsds where updated... I don't know if thats ok

So this pr is more of a first draft to fix the overall problem.

[SrTobi]

I also opened an issue few days ago #52.

[joaomoreno]

This is really clever, thanks! We will evaluate it for the next release!

[felixfbecker]

I think this is doing things way to complicated. NPM has an option to restrict packages in the list to only include production dependencies. Then all other folders should be excluded.

[joaomoreno]

@SrTobi, sorry for taking time to get to this. I think it is better here to use npm as a CLI instead of a dependency, simply because the transitive closure of dependencies of npm is huge.

Also, the definition file for Node upgrade is incorrect, since vsce needs to support 0.12.

Let me know if you want to update your PR. Otherwise, I will pick it up and use the CLI call.

[felixfbecker]

@joaomoreno That doesn't make much sense, npm is just an npm module itself. Since it is already installed on the user's system, its dependencies are installed already too. Since vsce is a global package just like npm, all the dependencies are already installed.

When you use the API, you a) spawn an unneeded process and have to do unneeded parsing, which means worse performance and b) you make a "global" dependency on the npm module without specifying it in package.json.

[felixfbecker]

@SrTobi did you implement the suggestions I raised about using the production-only parameter instead of custom parsing?

[SrTobi]

@fabienroyer nope I haven't touched the code since i opened the pr.

[SrTobi]

The idea behind using the npm module is, that the interface of the npm api is less likely to change in comparison to the output of npm (ok, ok it was also faster to implement 😄). This however was only my feeling and needs further investigation. I also do not like the version problem... this is something we definitely avoid if we use the npm cli.

Let me investigate, if the output of npm list has changed since v0.12 and continue the discussion afterwards.

[felixfbecker]

@SrTobi what do you mean with "the version problem"? And why exactly does vsce need to support an ancient Node version 0.12 when VS Code itself runs on 4.1.1?

[SrTobi]

By the "version problem" i mean the problem that we must support npm version 0.12. That might be problematic because if vsce uses npm version v0.12 but the system runs a much higher version of npm, the npm module might not be able to handle the node_module filesystem structure as well as other things.

@joaomoreno might know more about why we have to support version v0.12.

[SrTobi]

Ok, I changed the PR. Now it uses an child process and npm without the npm module.

[joaomoreno]

Awesome job, @SrTobi, thanks for the time and effort. Just published vsce@1.1.0 that has these changes.

@felixfbecker I just think there are still a bunch of people out there on Node < 4 installed in their systems, and I would like to keep compatible with those guys until a bit more time passes.

[SrTobi]

@joaomoreno have you tested it with older versions of npm?

[joaomoreno]

Runs fine with npm@2.14.16