timkindberg / jest-when

Jest support for mock argument-matched return values.
MIT License
737 stars 39 forks source link

Add .npmignore to keep published package slim #99

Closed ericcornelissen closed 2 years ago

ericcornelissen commented 2 years ago

Create the .npmignore file with an allowlist of files that should be published. I went with an "allowlist" approach to avoid files added in the future, as well as files that happen to exist at the time of publishing, from being included in the published packages.

I've never used yarn when publishing a package so I'm not 100% sure this will work, but from what I could find online it seems this should work.


For reference, this is what the installed package currently looks like:

$ npm ls jest-when
example@3.1.0 /path/to/example
└── jest-when@3.5.2

$ l node_modules/jest-when
.eslintignore
.eslintrc
LICENSE
package.json
README.md
src
stryker.conf.js
.tool-versions
.travis.yml

$ l node_modules/jest-when/src
when.js
when.test.js

And here's what it would look like after this change:

$ l node_modules/jest-when
LICENSE
package.json
README.md
src

$ l node_modules/jest-when/src 
when.js
timkindberg commented 2 years ago
ericcornelissen commented 2 years ago

This doesn't seem to exclude the .test.js file properly. I ran npm pack and it was still there. I'm also thinking like there is not much unnecessary files in the npm package, so this is not super important. But I'm willing to merge it if you get this test exclusion working.

My bad, I wrote this the wrong way around, it should've been:

# Ignore everything
*

# Publish from an allowlist
!src/**/*.js
src/**/*.test.js
!LICENSE
!package.json
!README.md

In my experience the best approach is to list exactly the files you want in package.files

Read For the love of god, don't use .npmignore

As far as I can tell using the allowlist approach suggested in this Pull Request (assuming I had done it correctly :sweat_smile:) avoids all the pitfalls mentioned in the article. I don't know if using files has any other benefits (doesn't seem like it from the docs), but which one to use seems like a toss-up to me.

I really appreciate your thought and effort on this, but I'm gonna just do the files field in package.json instead. Thanks for broaching the topic! I'll just push this change out with whatever the next version is, I'm not gonna do a release for this.

That works just fine for me :smile: I agree it's not necessary to create a release for this change.

joebowbeer commented 2 years ago

@ericcornelissen for future reference, npm pack is a good way to examine which files will be published