Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add package.json "files" entry #328

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

tbeseda
Copy link

@tbeseda tbeseda commented May 27, 2023

I realize using "files" in package.json essentially inverts how files are allowed into the published library, but it significantly reduces the size on disk for find-my-way (yes already small, but could be super small 😎 ).

npm notice === Tarball Details === 
npm notice name:          find-my-way                             
npm notice version:       7.6.2                                   
npm notice filename:      find-my-way-7.6.2.tgz                   
- npm notice package size:  79.4 kB
- npm notice unpacked size: 378.0 kB
- npm notice total files:   89
+ npm notice package size:  24.3 kB
+ npm notice unpacked size: 88.9 kB
+ npm notice total files:   14

The files that are packaged look correct to me, but here's a list

LICENSE                         
README.md                       
index.d.ts                      
index.js                        
lib/constrainer.js              
lib/handler-storage.js          
lib/http-methods.js             
lib/node.js                     
lib/pretty-print.js             
lib/strategies/accept-host.js   
lib/strategies/accept-version.js
lib/strategies/http-method.js   
lib/url-sanitizer.js            
package.json

fixes #327
Happy to hear feedback/make changes

Copy link
Collaborator

@ivan-tymoshenko ivan-tymoshenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. I like the idea of removing files from the package, but I would prefer to have an “exclude” behavior rather than “include”. Someone can add a file that would be not included, but tests would pass. @mcollina WDYT?

@tbeseda tbeseda mentioned this pull request May 27, 2023
@tbeseda
Copy link
Author

tbeseda commented May 27, 2023

I created #329 as an alternative that uses .npmignore 🍻

@Eomm
Copy link
Contributor

Eomm commented May 28, 2023

I have this saved quick answer about this topic:

Thank you for your interest in this topic. Please see the discussion in fastify/skeleton#42 (comment) for my reasoning against such a change.

But this repo is @delvedor domain 🙌🏼

@felixmosh
Copy link

Why not using .npmignore files to exclude tests / benchmark folders?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exclude test/ from package distribution
4 participants