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

suggestion: disable require-publishConfig for private: "true" packages #436

Open
bakkot opened this issue May 14, 2021 · 5 comments
Open
Labels
enhancement 👑 New feature or request

Comments

@bakkot
Copy link

bakkot commented May 14, 2021

Since the private setting prevents npm from attempting to publish a package, there's no need for publishConfig to be configured for those packages.

@dietergeerts
Copy link

There are other things that aren't needed, like a "files" field for example, because that is only used to pack and publish a package, but when it's private, it can't be. So it would be great if we can apply rules based on the fact if it's a private or "public" package.

@tclindner tclindner added the enhancement 👑 New feature or request label Nov 13, 2021
@tclindner
Copy link
Owner

Hey @bakkot and @dietergeerts what rules do you think we should do this far?

@bakkot
Copy link
Author

bakkot commented Nov 13, 2021

I actually disagree with the above comment - you can still npm pack a private package, so files is still a reasonable thing to include. private only prevents publishing, so the only rules it should disable are the ones which are only involved in publishing - i.e., require-publishconfig and valid-values-publishconfig.

@tclindner
Copy link
Owner

@dietergeerts are you comfortable with that?

@dietergeerts
Copy link

dietergeerts commented Nov 14, 2021

Well, I have several issues with this linter, let me explain further. We have a requirement to lint our package files, because there are certain things that are easily forgotten, for example, making sure only source files, and thus no dev files like tests, are included when publishing a package. Therefore, I needed to constraint the files field to certain values only AND make sure that the value to exclude test files was in there. So it always should something like this:

"files": ["lib", "tsconfig.json", "!.(test|test-*|stories).{??,???,????}"]

Where lib can also be src or bin, depending on what kind of package it is. We have decide on the rule that actual source files always need to be in a "main" directory, so that configurations like this, and linting/webpack/... can be made very easily and re-usable.

This is currently not possible with this linter, and thus we wrapped it in our own cli package, where we now run this linter, as some rules are still great, like the order of props, combined with ajv-cli, where we run the official schema and our own custom schema against the files.

While I do like the concept of rules that are clear and can easily put on/off, I also think it's too much dev work itself for the linter, and restrict its usage to certain scenarios like I explained above. As the package.json file is just json, it would be better to use JSON Schema internally, and then expose some general rules that can be configured, like 1 "require" rule, where the fields required can just be specified. On top of this, a custom schema can then be given for extra custom rules.

Another example of why it would make more sense for these general rules, is the fact that there are fields being added over time, like the "type" field that nodejs has added, although not official by npm, it's a field that is used by nodejs to determine if it should parse files like commonjs or the new module system. When these are added, we avoid work on the linter itself. Off course, when the internals are based on JSON Schema, it would be very easy to add rules, and maybe make it easier for devs with less knowledge about schemas to turn on rules though.

And then there are things like the one in this thread, rules that depend on other fields. When the package is private, some certain rules should be applied. Or when the package has a certain name, etc....

@bakkot , And while I do agree that technically you can still do pack on private packages, there is functionally no reason to do so, as it's a private package, so the files field is still functionally not needed.

Maybe it would be a good idea to do something like overrides, but based on certain fields instead of only the file-path name? That would meet several scenarios that are impossible now, and would avoid custom schemas that become very complex due to this check.

I'm just venting my thoughts here after setting up package.json linting for several times now, and always finding myself wrapping this linter with a custom CLI tool. I should mention that I'm not someone that wants linting rules just for it to look nice though, but for functional reasons/requirements, and for developer experience, to make the life of a dev easier.

I'm always open for more discussion on this topic, and if I find the time, I'm open to help improve this linter.

(as an example of thinking functionally, the rule names are off, some have the field name first, others as last, while the field name is the functional important things in them, so having that field name always first makes more sense from a functional standpoint, but not from a technical one though. )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 👑 New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants