npm install should remove `node_modules` line from installed `.gitignore` #5722

Closed
dominykas opened this Issue Jul 17, 2014 · 17 comments

Projects

None yet

5 participants

@dominykas

In most of the packages [that I'm using], .npmignore does not exist, so it is created by renaming the .gitignore. However, in some of them both -.npmignore and .gitignore exist. This basically leads back to the problem of #1886 - not all the dependencies get checked in for those of us who like to keep node_modules under version control.

Removing node_modules line in any of the installed .gitignore files is not a problem, since the whole parent node_modules is already ignored by those, who do not like checking it in.

This will save me running find . -name .gitignore | xargs grep -l 'node_modules' | xargs sed -i '' '/node_modules/d' with every install :)

@tjwebb
tjwebb commented Jul 18, 2014

.npmignore does not exist, so it is created by renaming the .gitignore

By what, or whom?

not all the dependencies get checked in for those of us who like to keep node_modules under version control.

Those of you who like to keep node_modules under version control should stop keeping node_modules under version control.

@dominykas

By what, or whom?

Been like that forever, by 2212118

Those of you who like to keep node_modules under version control should stop keeping node_modules under version control.

I expected at least one person to say exactly this, so I specifically have an answer prepared for this, but since it is a very angry one (it also includes the word "hipster" multiple times), I will just say something a little more positive: the beauty of using node is the freedom to do in the way you like it, not only in the way YOU like it. So, let's not open up this debate here.

@tjwebb
tjwebb commented Jul 18, 2014

Things in node_modules are binaries, and are managed by your package manager, not version control. npm is not git, and git is not npm. If you'd like to not follow established convention, then you get to deal with the inconveniences of not following established convention.

npm has covered this in their docs already, and specifically say to not do what you're trying to do:
https://www.npmjs.org/doc/faq.html#Should-I-check-my-node_modules-folder-into-git

the beauty of using node is the freedom to do in the way you like it, not only in the way YOU like it.

Feel free to fork npm and publish your own special version that behaves as you think it should.

@dominykas

Check node_modules into git for things you deploy, such as websites and apps.

I do exactly this.

You don't need to explain the trade offs of checking in/not checking in the dependencies together with my application.

Use npm to manage dependencies in your dev environment, but not in your deployment scripts.

I also do exactly this.

@dominykas dominykas changed the title from npm install should remove `node_modules` line from `.gitignore` to npm install should remove `node_modules` line from installed `.gitignore` Jul 18, 2014
@dominykas

Added one word to the subject. I hope this removes the confusion - only the .gitignore that comes with an installed module should be modified (and also only when an .npmignore exists together with it).

@tjwebb
tjwebb commented Jul 18, 2014

npm install does not care about .gitignore, nor should it. npm cares about npm. Why should it aggressively modify files it doesn't have any purview over? Should it also modify .svnignore and .hgignore and ... ?

You're also reading the docs wrong. If you keep node_modules in your repo, you have no reason to ever run npm install. Everything is already installed. That's why they recommend that for deployment-type things, so that your slug is already self-contained.

@dominykas

npm already modifies .gitignore (to rename it into .npmignore), specifically to cater for the node_modules in the repo scenario better.

I'm not sure we're having a constructive discussion here.

@tjwebb
tjwebb commented Jul 18, 2014

cp .gitignore .npmignore does not modify .gitignore.

None of this matters. This feature will not be implemented.

On Fri, Jul 18, 2014 at 5:20 AM, Dominykas Blyžė notifications@github.com
wrote:

npm already modifies .gitignore (to make it .npmignore), specifically to
cater for the node_modules in the repo scenario better.


Reply to this email directly or view it on GitHub
#5722 (comment).

@othiym23
Collaborator

My primary objection to seeing this in npm is that the behavior of .npmignore is already somewhat magical, and allowing this in would make it even more magical. Right now, you can use bundledDependencies to create an installable version of a package with all the relevant modules in there, regardless of what's in .gitignore / .npmignore. There's also gitignore pattern negation (!node_modules/*).

If you have a proposal to do this that doesn't involve destructively altering a file, or is generalized and not just a specific special case, we can talk about it. Your current proposal will address an irritation for you at the expense of potentially causing confusion and irritation for other people.

@dominykas

I'm not sure how bundledDependencies would help me - I have a private app, that does not get published in npm.

Why do you consider .npmignore to be magic? (Aside from the fact, that it might be created by mv .gitignore .npmignore)

Come to think of it... What does .npmignore do for modules that are installed? Is it not only a module publisher's feature? If so - does it even need to exist inside the modules post-publish? And if it doesn't - then does .gitignore even need to exist inside the published modules (A: it clearly doesn't, because it gets removed in 99% of the cases)?

As for "destructively altering"... yeah, there is that, but I'm not sure it is a problem. Like I've said before - the parent node_modules are already ignored (by the consuming app/module), and if they're not - then it's a clearly expressed intent, that child node_modules should not be ignored.

The .gitignore of the module has a purpose - it ignores files that should not be committed during that module's development. It's purpose, likely, isn't to ignore files during the modules consumption (although I'll admit I have no hard data on this).

As a consumer of some module in an app, I never want some module's sub-dependencies to be left out. Never. It is safer to leave the sub-dependencies in, given node's module resolution algorithm. Also do consider the fact that IF there's a .gitignore in one of my dependencies, it is a side effect of the author also having a .npmignore - which did come about as one of the solutions to NOT have that .gitignore and to NOT ignore the sub-node_modules.

@othiym23
Collaborator

Why do you consider .npmignore to be magic? (Aside from the fact, that it might be created by mv .gitignore .npmignore)

That's precisely what I consider magical – the first few times I encountered .npmignore files in my own published modules, I couldn't figure out where they came from. When I added .npmignore to a package with an existing .gitignore, I was surprised by the fact that they didn't combine. It's documented behavior, but it's counterintuitive. I'm not a fan of adding further complications here.

Come to think of it... What does .npmignore do for modules that are installed?

It doesn't interfere with git when checking in node_modules. It appears that this behavior was introduced precisely to better support users who check in node_modules.

And if it doesn't - then does .gitignore even need to exist inside the published modules (A: it clearly doesn't, because it gets removed in 99% of the cases)?

I've had need for both .gitignore and .npmignore before, when working around issues with bundledDependencies.

Let's restate the issue in terms of the problem you're trying to solve: you want to ensure that when you check in your application or module, all of its dependencies are included in git. Is that accurate?

If that's what you're looking for, a valid solution would be to rename .gitignore to .npmignore only if there isn't already a .npmignore, and to discard .gitignore from the package when both are present. This feels much more consistent (and less likely to go wrong) than filtering the contents of the file(s).

The issue there is that the version of the module you get on npm install is not as developer-friendly as what you get when you clone from git. I'm not sure that's a convincing argument one way or another, though. @isaacs, do you have an opinion on this?

@dominykas

Yes, that's the problem, and yes, I agree that leaving .gitignore out is a better idea than modifying it.

I'm not sure what you mean by "cloning from git" - if you mean installing by providing a git URL, then it is currently even more unfriendly, because it always keeps the .gitignore - this is where I had a couple of raised eyebrows when I saw gitignore change to npmignore, after installing a patched/updated module from npm having kept a git sourced variant for a while.

@dominykas

(Meh, mobile github has no edit)

As an aside, I have a feeling there are probably quite a few more differences when installing from git. Just a hunch, though. Would love to see all methods work the same, though.

@othiym23
Collaborator

I'm not sure what you mean by "cloning from git" - if you mean installing by providing a git URL

That's not how I meant it! I meant npm install <package> && cd node_modules/<package> vs git clone https://github.com/organization/<package>.git && cd <package> – that is, the developer experience of working with the package, not the user.

it is currently even more unfriendly, because it always keeps the .gitignore

This is a bug, and you can be fairly confident it will get addressed soon, because it bites me every single time I'm working on npm itself.

@wavded
wavded commented Jul 30, 2014

I second @dymonaz 's concern. I also check in modules for private projects. It is baked into NPM workflows for a lot of people (I opened the original case for this feature in 2011). IMO this is a bug preventing it from working the way it was intended when @isaacs and I discussed it. It sucks to have a features that works 99% of the time but really bites you that last 1% of the cases.

My gut is either remove the "magic" and we use negations or whatever (not my preferred, breaking), or make it work 100% of the time (I highly doubt people capitalize on having a few .gitignore files sitting around). Here's @isaacs words from the original case:

.gitignore files are never ever included in package tarballs. (A .gitignore file will be renamed to .npmignore if there's no .npmignore, or just silently excluded if there is one already.)

@dominykas

I believe this can now be closed? Not sure which version introduced that, but .gitignore is now [correctly?] never included in the published package (also it seems that packages installed directly from git also have this cleaned up properly?)

@othiym23
Collaborator
othiym23 commented Apr 3, 2015

Closing as resolved. Yes, both of those things have been changed in the last 6 months or so, as we try to ensure that git dependencies are at least handled consistently with respect to how npm caches regular directories and tarballs.

@othiym23 othiym23 closed this Apr 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment