ljharb / json-stable-stringify

MIT License
55 stars 11 forks source link

chore: don't pack `example/` and `test/` folders and other dev files #3

Closed mvayngrib closed 1 year ago

mvayngrib commented 1 year ago

currently this package includes many files that aren't needed at runtime. This PR npmignores them image

Test Plan

run npm pack. You should see only these files packed

npm notice === Tarball Contents === 
npm notice 592B   .github/FUNDING.yml            
npm notice 6.6kB  CHANGELOG.md                   
npm notice 1.1kB  LICENSE                        
npm notice 4.1kB  README.md                      
npm notice 2.1kB  index.js                       
npm notice 38.8kB json-stable-stringify-1.0.2.tgz
npm notice 2.0kB  package.json       
mvayngrib commented 1 year ago

@ljharb thanks for the quick response! Could u elaborate on why that's necessary? I imagine this can bloat node_modules quite a bit for other projects (maybe not json-stable-stringify/jsonify specifically). Can't the consumers who want to check tests clone the github repo and check out the tag (and compare files to npm contents if they really need to)?

ljharb commented 1 year ago

I don't particularly care about bloated node_modules; disk space is infinite and free.

Cloning the repo requires an internet connection, and, since repos are mutable (like this package's original repo), not guaranteed to exist at all.

mvayngrib commented 1 year ago

haha understood. What brought me here was actually a different reason. We audit all lines of code that make it to build machines, so this made audit pretty painful. Your module your choice of course

ljharb commented 1 year ago

That seems like a really impractical policy, given that tons of code will exist in node_modules that doesn't end up in production, and is thus irrelevant.