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

chore: use "files" instead of .npmignore #4405

Merged
merged 5 commits into from
Sep 27, 2023
Merged

chore: use "files" instead of .npmignore #4405

merged 5 commits into from
Sep 27, 2023

Conversation

regseb
Copy link
Contributor

@regseb regseb commented Sep 7, 2023

Use property files instead of .npmignore file to publish only the files you need, lighten packages, and reduce the risk of publishing a useless file.

Publishing what you mean to publish: There’s an even better way of controlling exactly which files are published with your package: whitelisting with the files array.

List of files that will no longer be published with this PR:

  • api, package reduced from 62.4ko to 34.2ko (54.8%)
    • src-generated/
    • src/
    • .mocharc.cjs, the file has been renamed from .mocharc.jsonc to .mocharc.cjs, but the name hasn't been changed in the .npmignore. 051ec93
    • CHANGELOG.md, this file is no longer included by default since npm v7.12.0: Do not force include history, changelogs, notice
  • core
    • src/
    • .mocharc.cjs
    • CHANGELOG.md
  • cucumber-runner
    • src-generated/
    • src/
    • .editorconfig
    • .eslintignore
    • .mocharc.cjs
    • prettierrc
    • CHANGELOG.md
  • grunt-stryker
    • CHANGELOG.md
  • instrumenter
    • src/
    • .mocharc.cjs
    • CHANGELOG.md
  • jasmine-runner
    • src-generated/
    • src/
    • .mocharc.cjs
    • CHANGELOG.md
  • jest-runner
    • src-generated/
    • src/
    • .mocharc.cjs
    • CHANGELOG.md
  • karma-runner
    • src-generated/
    • src/
    • .mocharc.cjs
    • CHANGELOG.md
  • mocha-runner
    • src-generated/
    • src/
    • .mocharc.cjs
    • CHANGELOG.md
  • tap-runner
    • src-generated/
    • src/
    • .mocharc.cjs
    • CHANGELOG.md
  • typescript-checker
    • src-generated/
    • src/
    • .mocharc.cjs
    • CHANGELOG.md
  • util
    • src/
    • .mocharc.cjs
    • CHANGELOG.md
  • vitest-runner
    • src-generated/
    • src/
    • .mocharc.cjs
    • CHANGELOG.md

@regseb regseb changed the title chore: use files instead of .npmignore chore: use "files" instead of .npmignore Sep 8, 2023
Copy link
Member

@nicojs nicojs left a comment

Choose a reason for hiding this comment

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

Hi @regseb thanks for participating here.

I agree with all files, except maybe src and src-generated. I started including them when vscode started logging warnings for missing sources of sourcemaps (also in node_modules). I'm pretty sure an issue was also raised, but I can't seem to find it.

What's your opinion on it?

@regseb
Copy link
Contributor Author

regseb commented Sep 24, 2023

I've added the src/ and src-generated/ directories.

It's a first step to replace .npmignore by "files". The next step will be to see about source maps, but I don't know how that works.

@nicojs nicojs merged commit f14e789 into stryker-mutator:master Sep 27, 2023
7 checks passed
@nicojs
Copy link
Member

nicojs commented Sep 27, 2023

Thanks!

@regseb regseb deleted the files branch September 27, 2023 13:20
@ericcornelissen
Copy link
Contributor

I noticed in the diff of v7.1.1 -> 7.2.0, which included this change, that there's actually a bunch of files newly included that don't need to be (even though the release line reads "ignore unused files"). Most notably test(/spec) files.

Primarily I wanted to raise the above. But I also wanted to share my perspective on this matter. Personally, I tend to favor a whitelisted-style .npmignore file (example) over the "files" array because it makes managing complex sub-folder rules a bit easier to manage (also for source maps) while providing the same benefits mentioned/referenced here.

@regseb
Copy link
Contributor Author

regseb commented Oct 3, 2023

@ericcornelissen Thanks for the feedback. There's a problem in my pull request. The "dist/" directories contain test files and tsbuildinfo. I'll make a new pull request to correct these problems.


For .npmignore / "files", I prefer "files" because :

  • the list of files to be published is in the same file as the "exports", "main", "bin" and "types" properties.
  • it's a positive list, whereas .npmignore is a negative list (Avoid negative conditionals).

All .npmignore features can be used in "files":

File patterns follow a similar syntax to .gitignore, but reversed: including a file, directory, or glob pattern (*, **/*, and such) will make it so that file is included in the tarball when it's packed.

Here's the equivalent of your example with "files":

{
  "files": [
    "docs/",
    "src/",
    "CHANGELOG.md",
    "index.cjs",
    "index.d.cts",
    "index.d.ts",
    "index.js",
    "package.json",
    "LICENSE",
    "README.md",
    "SECURITY.md",
    "testing.cjs",
    "testing.d.cts",
    "testing.d.ts",
    "testing.js"
  ]
}

@ericcornelissen
Copy link
Contributor

Good to know the "files" array supports the same expressions as .npmignore - in that case it's a matter of taste/preference 👍

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.

3 participants