-
Notifications
You must be signed in to change notification settings - Fork 213
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
Ignore dependencies of devDependencies #52
Comments
Very annoying! The docs state:
But when I check the installed extension:
See all the "extraneous" packages installed, those are dev-dependencies-dependencies. |
Here is a workaround, put this is package.json scripts: "vscode:prepublish": "npm install && npm run compile && npm prune --production" Before publishing, it will install everything, then compile, and at the end remove all devDependencies. Unfortunately, there is no postpublish hook, so you will have to run |
Great idea 👍 |
That is really cool, @felixfbecker, didn't know about |
Not really a fix, though. Perhaps something like this (opened before I realized there was a separate repo for vsce): microsoft/vscode#2114 |
I dont think reinstalling on publishing is the right solution. |
@felixfbecker even better, yes. I was not aware of that option. I'm assuming that eliminates any cross-dependencies (i.e. same package indirectly required by both |
Good point. So then the other way around, include only from node_modules what's in |
This is pretty epic, didn't know about this either. Would you like to submit a PR? 👍 |
While it doesn't solve the problem in the save way, a cursory glance seems like it would also work (an existing PR): #58 |
Hmm why not combine both solutions? In fact the code in my pr also uses |
@SrTobi That was my idea :) Of course it makes sense to use the NPM programmatic API instead of the CLI. But we shouldn't reimplement what NPM already provides. It is just important that we parse the production packages and only include them, instead of parsing the dev packages and exclude those. Otherwise you might have packages ignored that were both in your dev-dependencytree and your production-dependencytree (think about it, almost everyone depends on lodash somewhere in their dependency chain). |
Hmmm in fact I am actually doing this. I take all productive dependencies and extension files and then do the normal exclusion process (of course without the devDependencies). The idea to let npm output only productive dependencies instead of finding them by ourselves is pretty nice though. |
Fixed by #58 |
Hi, I am still facing this issue for an extension I wrote. "vscode:prepublish": "npm install && npm run compile && npm prune --production" which creates a much smaller file. I am using |
I did some debugging and it is definitely a npm problem. npm simply ignores the |
I also checked the npm command we are currently using (in https://github.com/Microsoft/vscode-vsce/blob/master/src/npm.ts#L4) and I think the
|
I opened an issue for the |
@SrTobi You need to specify depth because depth could be set through npm user config (I always set it to 0 to avoid the cluttered trees). But of course it should be something like 99999, not 0. |
ahh ok, didn't know about that. Then we should indeed set it to 9999. |
* Simplify command invocations are npm scripts * Add shortcuts for running these commands from VS Code * Use shx to run scripts in platform-agnostic manner * Add scripts to help with packaging and publishing of .vsix * Add instructions on how to publish * Clarify how to run on Windows @W-4162004@
- Converted esbuild from devDependency to runtime dependency - Removed node_modules exclusions from packaging. It looks like the vscode packaging is "supposed" to exclude dev dependencies. microsoft/vscode-vsce#52
Are dependencies of devDependencies ignored?
When I use
npm install
and thenvsce ls
a lot of node_modules are still listed. the devDependencies are ignored but their dependencies are not.Wouldn't it be better to include all production dependencies and their production dependencies and ignore all other modules?
The text was updated successfully, but these errors were encountered: