search

uuidjs / uuid

Generate RFC-compliant UUIDs in JavaScript
MIT License
14.65k stars 904 forks source link

fix: exclude tests from published package #840

Closed kyletsang closed 2 days ago

kyletsang commented 1 week ago

Current versions of v11 are including the tests in the published package.

This PR fixes that by preventing transpilation of the test folder in the root tsconfig and uses a test-specific tsconfig which does transpile test files. The tsconfig.test.json only targets esm since the original test command uses that folder only. Declarations are not emitted because they are not used.

broofa commented 6 days ago

Hey, thanks for catching this. I agree removing the test files probably makes sense. (That said I vaguely recall a conversation with someone a few years back making the case for including them...? But I don't recall when / where and I can't make a good case for keeping them.)

Instead of having a separate build pipeline for the test files - which has a whiff of code smell to it - let's add an .npmignore file with the necessary incantation(s) for excluding the test dir(s).

kyletsang commented 5 days ago

Hey @broofa, I looked over this today with the npmignore and ran into this from the docs:

At the root of your package it will not override the "files" field, but in subdirectories it will.

I wasn't able to get the npmignore to exclude the test dir. Another approach would be to switch out the "dist" entry in files with a glob like "dist/{cjs,cjs-browser,esm,esm-browser}/*.{js,d.ts}".

Thoughts?

broofa commented 4 days ago

Oof, sorry. 'Didn't realize that .npmignore was so ... ineffective. 😞

It turns out glob negations work in the files field so let's please do the following, instead:

  1. Revert the current changes.
  2. Change package.json#files to be as follows:
  "files": [
    "dist",
    "!dist/**/test"
  ]

And, yes, remove the *.md entries that are currently there. npm includes README and LICENSE by default, and the CHANGELOG and CONTRIBUTING docs don't need to be in the published package. This'll cut the package size from ~1.2M -> 0.8M.

BTW, npm pack --dry-run will show you which files will be published. (In case you weren't aware of that.) I've tested the change above and it works for me, but please verify and let me know if you see any issues.

kyletsang commented 4 days ago

Confirmed on my end, everything is as expected. Thanks!