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

Explicitly call out files in package.json #87

Closed
wants to merge 1 commit into from

Conversation

hildjj
Copy link

@hildjj hildjj commented Oct 29, 2024

Explicitly call out which files should be included in the published NPM package. This will keep tests, benchmarks, examples, and other non-essential files from making the package larger than it needs to be. The GitHub repository is correctly marked in the package file, so any user that wants these things
can have ready access to them online.

README.md and LICENSE are always included.

Fixes #78.

New output of npm pack --dry-run:

npm notice
npm notice 📦  @fastify/send@3.1.1
npm notice Tarball Contents
npm notice 1.2kB LICENSE
npm notice 8.6kB README.md
npm notice 513B index.js
npm notice 431B lib/collapseLeadingSlashes.js
npm notice 446B lib/containsDotFile.js
npm notice 416B lib/contentRange.js
npm notice 633B lib/createHtmlDocument.js
npm notice 443B lib/createHttpError.js
npm notice 314B lib/isUtf8MimeType.js
npm notice 636B lib/normalizeList.js
npm notice 2.7kB lib/parseBytesRange.js
npm notice 849B lib/parseTokenList.js
npm notice 17.0kB lib/send.js
npm notice 1.4kB package.json
npm notice 5.0kB types/index.d.ts
npm notice 1.5kB types/index.test-d.ts
npm notice Tarball Details
npm notice name: @fastify/send
npm notice version: 3.1.1
npm notice filename: fastify-send-3.1.1.tgz
npm notice package size: 12.5 kB
npm notice unpacked size: 42.1 kB
npm notice shasum: a4eb816822048cb0e0cfa0ab4ecb7595cc9d4ae2
npm notice integrity: sha512-aeMeRF+iaGR7V[...]Qia9fG83/aGag==
npm notice total files: 16
npm notice
fastify-send-3.1.1.tgz

I tested this to ensure I got all of the important files by:

  • npm pack
  • mkdir /tmp/foo
  • cd /tmp/foo
  • npm init -y
  • npm install .../fastify-send-3.1.1.tgz
  • create server.js from docs
  • node server.js
  • (in another tab) curl to test

Note that there is not a "benchmark" script in package.json, so I haven't run npm run benchmark as requested below.

Checklist

in the published NPM package.  This will keep tests,
benchmarks, examples, and other non-essential files
from making the package larger than it needs to be.
The GitHub repository is correctly marked in the
package file, so any user that wants these things
can have ready access to them online.

Fixes #78.
@Uzlopak
Copy link
Contributor

Uzlopak commented Oct 29, 2024

It is consensus between the fastify maintainers, that we include test files and stuff.

The solution for that issue with the test is to create that file when testing.

@Uzlopak Uzlopak closed this Oct 29, 2024
Copy link
Member

@jsumners jsumners left a comment

Choose a reason for hiding this comment

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

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

Also see https://www.youtube.com/watch?v=D18jlN2JNE0.

@hildjj
Copy link
Author

hildjj commented Oct 29, 2024

I strongly disagree with your reasoning. Using directories in the files attribute as I've done here means that there is very little maintenance required in practice.

However, I respect your right to do whatever you want with your projects.

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.

Folder name with emoji causes compression errors.
3 participants