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

Project status: picking back up! #28

Closed
11 tasks done
JoshuaKGoldberg opened this issue Sep 13, 2023 · 8 comments
Closed
11 tasks done

Project status: picking back up! #28

JoshuaKGoldberg opened this issue Sep 13, 2023 · 8 comments
Assignees

Comments

@JoshuaKGoldberg
Copy link
Owner

JoshuaKGoldberg commented Sep 13, 2023

👋 Hi! I'm going to be helping out with maintenance of the project. Pinning this issue to explain project context & next steps.

Project Context

This is a plugin for package.json-file-specific logic in ESLint. Other tools exist for doing that (https://github.com/henrikjoreteg/fixpack, https://github.com/tclindner/npm-package-json-lint), but the advantage of this one is operating solely within ESLint. That means it benefits from all the tooling (granularly configurable rules, fixes, suggestions, etc.) that come with ESLint. And I personally am excited about getting this plugin to feature parity with other ecosystem equivalents so I can reduce the number of config files in my repositories.

Thanks to @zetlen for first creating eslint-plugin-package-json back in 2018!

Next Steps

Here's what I'm going to do with this repository:

It's an exciting time to be an ESLint plugin! 🥳


Edit (October 17th, 2023): I've started merging #39 and other PRs that drastically change the library. They're available on a 0.3.x line in npm, tagged as beta.


Please do post feedback if you have any questions or suggestions. Cheers all! ❤️

@Zamiell
Copy link
Contributor

Zamiell commented Sep 13, 2023

I think the readme should briefly discuss prettier-plugin-packagejson. I think for most use-cases, end-users will need either prettier-plugin-packagejson or eslint-plugin-package-json. Which is "better"? A short discussion:

Fixing: Tied

We don't want to have to manually reorganize fields in our package.json file. As soon as we save the file, it should just be automatically ordered.

Both solutions provide this, since a typical workflow in JavaScript/TypeScript projects is to run prettier --write and/or eslint --fix as soon as the file is saved.

Config complexity: Tied

  • prettier-plugin-packagejson is a plugin of Prettier, so using it will cause your Prettier config to get more complicated.
  • eslint-plugin-package-json is a plugin of ESLint, so using it will cause your ESLint config to get more complicated.

(Nit: eslint-plugin-package-json should automatically enable the plugin inside of the recommended config. Doing this is idiomatic in ESLint plugins AFAIK and it would reduce the boilerplate. I can open a separate issue for that if you want.)

Obviously, the above comparison assumes you are already using both Prettier and ESLint in your JavaScript/TypeScript project. If you are not using Prettier (booooo), then obviously eslint-plugin-package-json is the winner here, and the rest of the discussion below is pointless.

IDE Errors: prettier-plugin-packagejson wins

We don't want to see yellow squiggly lines in our editor when formatting errors exist.

Prettier recommends that you do not use eslint-config-prettier. This is for several reasons, but most importantly, the typical workflow with Prettier is that the file automatically formats itself as soon as you save the file, so seeing your file fill up with yellow squiggly lines as you type things is both annoying and pointless. In other words, formatting related concerns don't need to be shown as an IDE warning/error.

The order-properties rule and the sort-collections rule as formatting concerns, so for the same reason, we don't want these being shown in our IDE.

Tool Fit: prettier-plugin-packagejson sort-of wins

The order-properties rule and the sort-collections rule are formatting rules. Formatting concerns should be handled by Prettier. Thus, it makes more sense to use Prettier for this purpose and not ESLint.

However, Prettier does not all-out win here because the valid-package-def rule does not have to do with formatting and has to do with correctness. Furthermore, this rule has no analog functionality in prettier-plugin-packagejson. But more on this rule later.

File Extension Fit: prettier-plugin-packagejson wins

Prettier is a better fit based on the file extension. Prettier already formats all the JSON files in your repository, whereas ESLint is normally only configured to run on JS/TS files. Because of this, there is a foot-gun relating to this ESLint plugin:

  • When using Prettier, a repository will typically have a script like this: prettier . (i.e. to run on every file in the repo, because we want Prettier to format e.g. all of our TypeScript files, all of our Markdown files, and so on)
  • When using ESLint, a repository may have a script like this: eslint ./src/**.ts (i.e. to run on every TypeScript file in the "src" directory)

In this case, after installing and enabling eslint-plugin-package-json, nothing would be working, until we identified that we had to change how we were invoking ESLint itself. Kind of a nasty problem!

Validating field contents: eslint-plugin-package-json sort-of wins

The valid-package-def rule validates field contents and there is no analog functionality in prettier-plugin-packagejson.

But the idiomatic way to validate JSON files is to simply add a @schema field. In the case of a "package.json" file, we would add:

{
  "$schema": "https://raw.githubusercontent.com/SchemaStore/schemastore/master/src/schemas/json/package.json",
}

And now we would get tab-complete and field validation inside VSCode.

Of course, it's possible to forgot to specify the schema in a new project, and then go on to have invalid fields in there. A solution to that might be to make a new ESLint rule that looks for a @schema field. But at that point, if you are going to have ESLint start examining your JSON files, then you might as well have ESLint do the formatting + validation too. (Although that solution has the disadvantage of not having tab-complete inside of the "package.json" file itself.)

My Conclusion

IMHO, I don't think that the valid-package-def rule provides very much value. I've known about JSON @schema fields for a few years, and it has never occurred to me to put a @schema inside of a "package.json" file specifically. Doing that is not necessarily a bad idea per say: I love tab-complete inside JSON files! But it's more that:

  1. package.json files are usually auto-generated by npm init or similar tooling
  2. package.json files are rarely touched by a human manually editing the file
  3. package.json files are so common that the format of the fields is widely known

If we disregard the valid-package-def rule existing for a moment, then I think prettier-plugin-packagejson is the clear winner for the reasons that I outlined above.

Would be interested in getting your thoughts on this Josh and then maybe provide a summary of this discussion in the readme. Would it make sense to recommend that people use prettier-plugin-packagejson instead?

@Zamiell
Copy link
Contributor

Zamiell commented Sep 19, 2023

More info about valid-package-def: Apparently VSCode special cases any file named "package.json" and automatically inserts the schema, which provides both auto-complete and type validation (e.g. Incorrect type. Expected one of object, string.). Thus, the rule provides less value than what I speculated in my previous comment. It would only help 1) e.g. vim users and 2) validation of existing code in CI.

@piranna
Copy link
Contributor

piranna commented Sep 19, 2023

More info about valid-package-def: Apparently VSCode special cases any file named "package.json" and automatically inserts the schema, which provides both auto-complete and type validation (e.g. Incorrect type. Expected one of object, string.). Thus, the rule provides less value than what I speculated in my previous comment. It would only help 1) e.g. vim users and 2) validation of existing code in CI.

I'm particulary interested on this use case. VSCode does that validation by checking about the schema, but I don't know where is defined.

@Zamiell
Copy link
Contributor

Zamiell commented Sep 19, 2023

@piranna You can find the package.json schema here: https://raw.githubusercontent.com/SchemaStore/schemastore/master/src/schemas/json/package.json
(I included it in my previous comment.)

As for what VSCode officially uses, I'm not sure. I cloned the repo and searched for the string "package.json", but I didn't find much.

@piranna
Copy link
Contributor

piranna commented Sep 19, 2023

Then, we can use the schema validation as one of the rules :-)

@JoshuaKGoldberg
Copy link
Owner Author

JoshuaKGoldberg commented Sep 20, 2023

Thanks for the discussion! I filed #42 with a comparison table showing that there are quite a few non-formatting concerns this plugin can grow to handle. #11 already added a local-dependency rule that is pretty solidly in the side of a linter, not a formatter.

(Nit: eslint-plugin-package-json should automatically enable the plugin inside of the recommended config. Doing this is idiomatic in ESLint plugins AFAIK and it would reduce the boilerplate. I can open a separate issue for that if you want.)

Err, I don't follow the change request, but sure! A new issue would be great.

Prettier recommends that you do not use eslint-config-prettier

Is that a typo? Did you mean that Prettier recommends you do use eslint-config-prettier?

using Prettier
...
The order-properties rule and the sort-collections rule as formatting concerns

I think there's still a use case for having ordering rules as part of this plugin. There are formatters (e.g. dprint) that might not have the same plugin extensions. Or in dprint's case, it's not optimal to run the more-priviledged-than-usual process for running Prettier. Filing an issue: #46.

File Extension Fit: prettier-plugin-packagejson wins

Many repos & docs have been using to all-inclusive globs such as eslint .. I think that that's become sort of a standard - especially as more and more folks use ESLint for more languages.

There's also the (not very severe) issue that folks tend to enable this plugin on all files right now. In #40 I'm updating the README.md to explicitly suggest an ESLint override.

$schema

Discussed in #41: it's not idiomatic for package.json files to have a $schema. So I don't think we'll be adding one here.

@Zamiell
Copy link
Contributor

Zamiell commented Sep 20, 2023

A new issue would be great.

Done.

Is that a typo? Did you mean that Prettier recommends you do use eslint-config-prettier?

Sorry, I meant to type eslint-plugin-prettier, not eslint-config-prettier.

@JoshuaKGoldberg JoshuaKGoldberg unpinned this issue Jan 20, 2024
@JoshuaKGoldberg
Copy link
Owner Author

Closing as it's been a few months since any discussion here. Things are chugging along. 2024 will be a big year for this plugin. 🙂

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

No branches or pull requests

3 participants