ember-cli / eslint-plugin-ember

An ESLint plugin that provides set of rules for Ember applications based on commonly known good practices.
MIT License
259 stars 201 forks source link

Lint package.json requirements with eslint-plugin-json-files #1721

Open NullVoxPopuli opened 1 year ago

NullVoxPopuli commented 1 year ago

With @kellyselden's PR here: https://github.com/kellyselden/eslint-plugin-json-files

I think it now makes sense to add some very important, yet presently missing lints for:

bmish commented 1 year ago

How do you detect an Ember app, engine, or addon if it doesn't have ember-app, ember-engine, or ember-addon? I assumed that keyword was the source of truth.

NullVoxPopuli commented 1 year ago

yeah the keyword is a source of truth, but when authoring a package.json manually (as may be common with v2 addons, for example), it's an easy thing to forget. When omitted, It leads to some weird build / runtime bugs, last I checked

bmish commented 1 year ago

I want to note that npm-package-json-lint exists and is pretty powerful and mature for these kind of checks, although since it's not an ESLint plugin, it's not something that can be incorporated into eslint-plugin-ember.

NullVoxPopuli commented 1 year ago

yeah, I'm thinking we want to integrate eslint-plugin-json-files so folks can just extend: ['ember'] in their .eslintrc, and have all the proper footgun-prevention checks occur

bmish commented 1 year ago

I think it's an open question whether package.json linting should be incorporated into eslint-plugin-ember. Pretty much all ESLint plugins are focused on linting a single type of file (and almost always JS/TS files only). I'm open to experimenting with this, but we may end up finding it deserves its own package so that each package can stay focused, and avoid unnecessarily tying two separate types of linting together.

It could possibly make sense to have a npm-package-json-lint-config-ember package (similar to npm-package-json-lint-config-default which just configures the rules from npm-package-json-lint for Ember.

NullVoxPopuli commented 1 year ago

my take is that linting is way too complicated for the average person, and any way we can make things simpler for folks, the better.

this could take form in updating the default lint configs -- but that's a hard conversation to have (too many cooks in the kitchen, imo)

bmish commented 1 year ago

The ember-cli blueprint already includes various JavaScript linters including eslint-plugin-ember, handlebars linting via ember-template-lint, and there's a proposal to add stylelint. I'm not sure why package.json linting necessarily belongs in the Ember-JavaScript-focused linting package. Just to suggest an alternative, package.json linting might deserve to be an additional type of linting added to the ember-cli blueprint, once there's a stable and worthwhile package available for that.

NullVoxPopuli commented 1 year ago

I started a thread in discord, if you want to help work out some details :sweat_smile: