Prune should not remove bundleDependencies. #4573

Closed
michaelgilley opened this Issue Jan 30, 2014 · 7 comments

Comments

Projects
None yet
5 participants
@michaelgilley

As is, npm prune will remove bundled dependencies. This issue can be fixed by adding a check for bundleDependencies in read-installed.js (although I'm unaware if this will create other side effects).

@michaelgilley

This comment has been minimized.

Show comment
Hide comment
@michaelgilley

michaelgilley Jan 30, 2014

This only occurs when bundledDependencies are not specified as dependencies. (Maybe specify this in the docs?

This only occurs when bundledDependencies are not specified as dependencies. (Maybe specify this in the docs?

@rlidwka

This comment has been minimized.

Show comment
Hide comment
@rlidwka

rlidwka Feb 1, 2014

Contributor

Why did you close it?

I think it is a bug, bundled deps should not be removed because they are used when packing.

Contributor

rlidwka commented Feb 1, 2014

Why did you close it?

I think it is a bug, bundled deps should not be removed because they are used when packing.

@michaelgilley

This comment has been minimized.

Show comment
Hide comment
@michaelgilley

michaelgilley Feb 1, 2014

Bundled deps should also be listed in dependencies or devDependencies. This becomes an issue only when they aren't. 

Although it should be understood that a bundled dependency is a dependency by default. So this could be an issue.

On Sat, Feb 1, 2014 at 1:30 AM, Alex Kocharin notifications@github.com
wrote:

Why did you close it?

I think it is a bug, bundled deps should not be removed because they are used when packing.

Reply to this email directly or view it on GitHub:
#4573 (comment)

Bundled deps should also be listed in dependencies or devDependencies. This becomes an issue only when they aren't. 

Although it should be understood that a bundled dependency is a dependency by default. So this could be an issue.

On Sat, Feb 1, 2014 at 1:30 AM, Alex Kocharin notifications@github.com
wrote:

Why did you close it?

I think it is a bug, bundled deps should not be removed because they are used when packing.

Reply to this email directly or view it on GitHub:
#4573 (comment)

@rlidwka

This comment has been minimized.

Show comment
Hide comment
@rlidwka

rlidwka Feb 2, 2014

Contributor

Bundled deps should also be listed in dependencies or devDependencies.

Why is that? I can add a private package that's not installable from anywhere, and use it as bundledDependency just as well.

Contributor

rlidwka commented Feb 2, 2014

Bundled deps should also be listed in dependencies or devDependencies.

Why is that? I can add a private package that's not installable from anywhere, and use it as bundledDependency just as well.

@michaelgilley

This comment has been minimized.

Show comment
Hide comment
@michaelgilley

michaelgilley Feb 3, 2014

For now, if you just list the package in one of the dependency hashes also everything will run as expected.

However, it is a legitimate question why bundleDependencies are not treated as their own kind of dependency.

I'm reopening this for official comment.

For now, if you just list the package in one of the dependency hashes also everything will run as expected.

However, it is a legitimate question why bundleDependencies are not treated as their own kind of dependency.

I'm reopening this for official comment.

@michaelgilley michaelgilley reopened this Feb 3, 2014

@rlidwka rlidwka referenced this issue in npm/read-installed Mar 6, 2014

Closed

do not mark bundled deps as extra #18

rlidwka added a commit to rlidwka/yapm that referenced this issue Mar 7, 2014

mrdanimal added a commit to mrdanimal/debowerify that referenced this issue Mar 25, 2014

mrdanimal added a commit to mrdanimal/debowerify that referenced this issue Mar 25, 2014

@othiym23 othiym23 added the bug label Sep 26, 2014

@busches

This comment has been minimized.

Show comment
Hide comment
@busches

busches Sep 26, 2016

@othiym23 is the official comment that this is a bug then? :) @michaelgilley makes a good point that if the dependency was listed in dependencies or devDependencies then npm prune would not remove them.

busches commented Sep 26, 2016

@othiym23 is the official comment that this is a bug then? :) @michaelgilley makes a good point that if the dependency was listed in dependencies or devDependencies then npm prune would not remove them.

@npm-robot

This comment has been minimized.

Show comment
Hide comment
@npm-robot

npm-robot Jun 19, 2017

We're closing this issue as it has gone thirty days without activity. In our experience if an issue has gone thirty days without any activity then it's unlikely to be addressed. In the case of bug reports, often the underlying issue will be addressed but finding related issues is quite difficult and often incomplete.

If this was a bug report and it is still relevant then we encourage you to open it again as a new issue. If this was a feature request then you should feel free to open it again, or even better open a PR.

For more information about our new issue aging policies and why we've instituted them please see our blog post.

We're closing this issue as it has gone thirty days without activity. In our experience if an issue has gone thirty days without any activity then it's unlikely to be addressed. In the case of bug reports, often the underlying issue will be addressed but finding related issues is quite difficult and often incomplete.

If this was a bug report and it is still relevant then we encourage you to open it again as a new issue. If this was a feature request then you should feel free to open it again, or even better open a PR.

For more information about our new issue aging policies and why we've instituted them please see our blog post.

@npm-robot npm-robot closed this Jun 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment