publint icon indicating copy to clipboard operation
publint copied to clipboard

Recognize more files that should not be included in npm package

Open rubiesonthesky opened this issue 10 months ago • 2 comments

Files that could be added to commonInternalPaths. Or maybe it could be it's own rule? I think it would be also nice if the current rule could inform what are the files that triggered the warning. :)

CI files

Test and local tooling files

  • New eslint config files: eslint.config.js, eslint.config.mjs and other variants
  • .eslintignore (e.g. saucelabs)
  • .prettierignore(e.g. saucelabs)
  • .huskydir
  • e2e dir
  • .taprc.yaml
  • .gitattributes, .gitignore, .gitkeep (e.g. danger)
  • gulpfile.js
  • jshintrc
  • .clang-format
  • tslint.json
  • fixtures dir
  • .vscode dir (e.g. ms-rest)
  • yarn.lock
  • tsconfig.tsbuildinfo (e.g. geckodriver)
  • tsconfig.spec.json (e.g. danger)

npm

  • .npmignore

Misc

  • CONTRIBUTING.md
  • code-of-conduct.md
  • CODEOWNERS
  • Dockerfile (e.g. swagger2openapi)
  • .release-it.json (e.g. danger)

The tool could also warn about missing README.md file.

rubiesonthesky avatar Feb 02 '25 10:02 rubiesonthesky

Thanks for tracking the list! I'll try to respond to a set of items that I agree / prefer-not-to below:

For "CI files" and "Misc", I think all of them would be nice to be checked. Except .release-it.json, I think it's too specific of a tool to add.

For .npmignore, I think package managers should be ignoring that automatically? Maybe it's only for latest package managers, but I think it's probably ok to skip checking it for now.

For "Test and local tooling files":

  • New eslint config files: eslint.config.js, eslint.config.mjs and other variants ✅
  • .eslintignore (e.g. saucelabs) ✅
  • .prettierignore(e.g. saucelabs) ✅
  • .huskydir ✅
  • e2e dir ✅ (a bit risky but could try)
  • .taprc.yaml ❌ (I prefer to stick with a popular list of tooling for now)
  • .gitattributes, .gitignore, .gitkeep (e.g. danger) ✅ (except .gitkeep, i think there may be cases to preserve the directory structure with that file when publishing)
  • gulpfile.js ❌ (I prefer to stick with a popular list of tooling for now)
  • jshintrc ❌ (I prefer to stick with a popular list of tooling for now)
  • .clang-format ❌ (I prefer to stick with a popular list of tooling for now)
  • tslint.json ❌ (I prefer to stick with a popular list of tooling for now)
  • fixtures dir ❌ (a bit risky as it might be used for normal cases)
  • .vscode dir (e.g. ms-rest) ✅
  • yarn.lock ✅ (and other lockfiles)
  • tsconfig.tsbuildinfo (e.g. geckodriver) ✅
  • tsconfig.spec.json (e.g. danger) ❌ (I'm not sure we need this for now)

I'm happy to discuss more about the ❌ ones, but one thing about intention of the rule is that it only checks if "files" is not set. So if a popular pattern is found, e.g. .eslintignore, it's also highly likely that other unpopular patterns are also published, and the author can go ahead and add "files" to settle it altogether.

If we ignore "files" and always check the patterns, it can be risky and cause false positives, for example, create-* CLI that contains template files. But maybe we can have a different set of checks for files that have no reason to be published at all, like lockfiles. (under the same rule)


I think it would be also nice if the current rule could inform what are the files that triggered the warning. :)

Agree too. I think it was just tricky to figure out the right message to show nicely. Can be tackled separately of this issue though.


Something else I just recalled is that it'd also be nice if it checks some files recursively, e.g. *.test.js files. But this can also be tackled separately.

bluwy avatar Feb 05 '25 10:02 bluwy

Additionally maybe some files here should be checked: image

bluwy avatar Sep 25 '25 00:09 bluwy