eslint-plugin-ember icon indicating copy to clipboard operation
eslint-plugin-ember copied to clipboard

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

Open NullVoxPopuli opened this issue 3 years ago • 8 comments

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:

  • package.json
    • for all addons
      • keywords must have ember-addon
    • v2 addons
      • require an addon-main entry and the file must exist
      • ember-addon (or ember) entry must exist
        • version must be set to 2
        • addon-main must be present in exports
        • app-js is optional, but must point at files present in the exports
      • a the output / dist folder must include at least one entry in exports (name of dist should be configurable)
      • if a v2 addon is "browser-only", we don't need an engines entry
      • forbid dependencies (things common in v1 addons)
        • ember-cli-htmlbars
        • ember-cli-babel
      • recommend peerDependencies (+ devDeps):
        • ember-source

NullVoxPopuli avatar Dec 30 '22 17:12 NullVoxPopuli

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.

bmish avatar Dec 30 '22 17:12 bmish

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

NullVoxPopuli avatar Dec 30 '22 17:12 NullVoxPopuli

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.

bmish avatar Dec 30 '22 17:12 bmish

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

NullVoxPopuli avatar Dec 30 '22 17:12 NullVoxPopuli

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.

bmish avatar Dec 30 '22 19:12 bmish

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)

NullVoxPopuli avatar Dec 30 '22 19:12 NullVoxPopuli

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.

bmish avatar Dec 30 '22 19:12 bmish

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

NullVoxPopuli avatar Dec 30 '22 20:12 NullVoxPopuli