I think it will be nice if, before publishing, np checked whether the package has either:
files field in package.jsona .npmignore fileThis would help publishers make cleaner packages and reduce the bandwidth used when installing the resulting package.
I would actually advocate towards making sure there is a files field, as it's better IMO than using .npmignore, but that's just my opinion. A well designed .npmignore has the same result.
IssueHunt Summary
๐ ๐ I can't believe I didn't think of that. I'm like the biggest proponent of the files property.
How about failing if there's neither and warn about recommending files if there's a .npmignore?
I had the same thought :D
I'd be fine with that. I think you should you be able to force through the error with an option though.
I think you should you be able to force through the error with an option though.
Why? There's absolutely no reason to not use either files or .npmignore.
I agree with Sindre. It gives me the creeps to see test files being part of a shipped dependency...
I'm thinking of the case when you started a new repo and want/need to publish it quickly for some reason and you don't want to mess up the package by quickly adding a files property without checking it's right. Maybe you don't have the time to check it because it's very urgent and you'd need to make a PR to add the files field and get it approved by your team, etc...
Ok, maybe it's a bit farfetched, I agree. But other than this case, I don't see a good reason no.
I thought about letting this pass when the package is so simple that it would not publish anything extra anyway, but I'm not sure it's worth going through the trouble of implementing it for what seems like a very edgy case.
you want to publish your repo for the
I would suggest adding this functionality without an option. If someone comes up with a valid use case, we can always reconsider.
Yeah, let's do that. Anyway, if this case comes up, they could still use the original npm publish
The only thing that makes me a bit anxious is that of the two solutions, files and npmignore, both require that all team members remember to either add new files that should be ignored to npmignore (failure could in worst case result in uploaded secrets) or to add new files to files (failure to do so could in worst case result in broken code).
Currently I have opted to push for np internally at work as it's the most fool proof way of publishing new versions of npm packages โ with the running of tests and everything.
Will be hard though to test whether files includes everything it should or whether npmignore ignores everything that it should. Enforcing that ones always use one or another in a team that isn't used to working with them sounds like a recipe for broken packages.
I'm ๐ on adding a warning though, especially as a first step, to nudge people in this direction. If one could then somehow add a safety net to catch missing additions to either files or npmignore then I would have no problem with enforcing it either, but unfortunately I don't think such a safety net can't be constructed.
either add new files that should be ignored to npmignore (failure could in worst case result in uploaded secrets)
How is this worse than not using .npmignore though?
Will be hard though to test whether files includes everything it should
@sindresorhus It's worse as .npmignore overrides .gitignore, which hopefully everyone knows how to use, so anyone who expects a file ignored in .gitignore to be excluded from publishing (as is the default of npm) will accidentally be publishing files it shouldn't
Related: perhaps there can be a size check too/instead. Shipping a hundred 1KB files (like tests, usually) is not as bad as a 5MB package. To put things into perspective, all babel's tests take 750KB of disk space (although they are 4000 files)
@voxpelli gitignore might be ignored at some point:
_Stop using
.gitignoreas a fallback for.npmignorenpm/npm#12331_
@bfred-it Totally and that would solve my anxiety here that enforcing a mechanism like this on people not expecting a mechanism like this will result in them doing things that goes against their intentions.
But we're not there now so until we are I will be restrictive in adding either files or .npmignore to projects as the risk of someone forgetting to update them is too large and I would like it if np allowed me to keep it that way as all of the rest of np:s features helps people in delivering on their intentions rather than enforces things that might have them do the opposite.
So I'm :+1: on any nudging to raise awareness about this issue in the meanwhile though, but ๐ on any enforcing until a change like the one you mention has been implemented.
If we would enforce it, you can easily just add an empty .npmignore file, no? I'm not very enthusiastic about adding complex checks on file size etc. It will fail at some point. So if you want to work around this one, I would just suggest adding an empty .npmignore. At least, it makes you aware of the issues about not adding a files property or a .npmignore file.
@SamVerschueren Simply adding an empty .npmignore makes npm ignore the .gitignore and thus go against the expectations of all of those who are used to npm respecting the contents of .gitignore, so no, that would not help
@voxpelli That's a fair point. But instead of creating an empty .npmignore, you could copy the contents of .gitignore. Knowing that option, would that rid you of your anxiety? Would having an option to override this default behavior help otherwise?
Is there a verdict then?
It seems to me like for now, warning users about using .npmignore / not having it at all, and recommending to use the files property (because of the reasons mentioned above) would be a good way to get this landed in np. We can always change the warning to be an error.
@issuehunt has funded $60.00 to this issue.
@itaisteinherz Yes, let's start with a warning, and we can consider making it an error in the future if the warning works out well. I would show the warning at startup before the publish process begins.
I wonder if we could check with Git if any files were added since the last release, and if so, we could check if they match any patterns in files or .npmignore and if not, we could show them so users would be aware. It could help catch mistakes for both forgetting to add a file to the files field and forgetting to add a file to .npmignore. Thoughts?
I would show the warning at startup before the publish process begins.
@sindresorhus Even before the ui.js, or after that but before publishing begins?
@itaisteinherz I would show it before/above Publish a new version of github-username (current: 5.0.1).
The files field and .npmignore check was added in https://github.com/sindresorhus/np/pull/411.
We still need to do:
I wonder if we could check with Git if any files were added since the last release, and if so, we could check if they match any patterns in files or .npmignore and if not, we could show them so users would be aware. It could help catch mistakes for both forgetting to add a file to the files field and forgetting to add a file to .npmignore.
I read the discussion and i wanted to started working on this.
List up the files added since the last release, i use often for code-reviews.
If the last release has a git-tag, we could use the git diff command to find out the files, which were added since the last release.
I while ago I created a CLI tool called comprehensive-npmignore to do this.
https://www.npmjs.com/package/comprehensive-npmignore
It asserts that you have explicitly declared all files as either included or excluded.
It does not do anything with git, which makes sense in case you have non-versioned build artifacts that you must remember to declare.
Anyway, I see that #456 is almost done already, but I'll leave this here anyway since the implementation is a bit different. Maybe future versions of np will do something similar.
@sindresorhus has rewarded $54.00 to @bunysae. See it on IssueHunt
Most helpful comment
Why? There's absolutely no reason to not use either
filesor.npmignore.