Enforce rules based on "engines" field in package.json #281

pvdlg · 2018-01-07T09:09:32Z

This allow to enable specific rules/rules configuration based on the range defined in engines field in package.json or the nodeVersion option.

If nodeVersion is set to false then the old behavior is resumed: no Node version specific rules are enabled.

The following rules are enable:

promise/prefer-await-to-then for Node 8 and above
prefer-rest-params for Node 6 and above
prefer-spread for Node 5 and above
prefer-destructuring for Node 6 and above

Should I add more?

We could later add node/no-unsupported-features by passing the value opts.nodeEngine in the version option.
Unfortunately for now the version option of the rule accept only a specific version and not the range defined in engines.node or nodeEngine. That should be solved once mysticatea/eslint-plugin-node@846e677 get released. We could enable the rule then.

I also added a "Tips" for overriding the test when using AVA. Maybe we can make that automatic if the users have AVA in their dependencies?

Fix #266

sindresorhus · 2018-01-08T10:07:21Z

Maybe we can make that automatic if the users have AVA in their dependencies?

Yeah, let's do that.

pvdlg · 2018-01-08T14:22:09Z

AVA doesn't seems to expose an API to read its config to figure out the files option.

Do I have a convenient way to determine where are the AVA test files for the current user?

If no I can use the default that would be the list here

pvdlg · 2018-01-10T20:13:12Z

I added the default list of AVA files. Let me know you'd prefer something else.

sindresorhus · 2018-01-14T22:47:22Z

lib/options-manager.js

+	'prefer-destructuring': {
+		'6.0.0': ['error', {array: true, object: true}, {enforceForRenamedProperties: true}]
+	}
+};


Can you include https://github.com/sindresorhus/eslint-config-xo/blob/3f8179657736c4c078759416509ca9698e5321e6/esnext.js#L16 too?

Id didn't include it becasue it's deprecated. See https://eslint.org/docs/rules/prefer-reflect

Ah, I didn't realize. We can eventually fix that: sindresorhus/eslint-plugin-unicorn#142

I think there is a bunch of rules to add from eslint-plugin-unicorn and some of them should apply only for specific Node.js version. But that might be better to do that in a different PR.

sindresorhus · 2018-01-14T22:48:15Z

main.js

@@ -21,6 +22,7 @@ const cli = meow(`
 	  --space           Use space indent instead of tabs  [Default: 2]
 	  --no-semicolon    Prevent use of semicolons
 	  --prettier        Conform to Prettier code style
+	  --node-version    Range of Node version to support


Node => Node.js

sindresorhus · 2018-01-14T22:49:50Z

readme.md

@@ -27,6 +27,7 @@ Uses [ESLint](http://eslint.org) underneath, so issues regarding rules should be
 - No need to specify file paths to lint as it lints all JS files except for [commonly ignored paths](#ignores).
 - [Config overrides per files/globs.](#config-overrides)
 - Includes many useful ESLint plugins, like [`unicorn`](https://github.com/sindresorhus/eslint-plugin-unicorn), [`import`](https://github.com/benmosher/eslint-plugin-import), [`ava`](https://github.com/avajs/eslint-plugin-ava), [`node`](https://github.com/mysticatea/eslint-plugin-node) and more.
+- Automatically enable or disable rules based on the [engines](https://docs.npmjs.com/files/package.json#engines) of your `package.json`.


Automatically toggles rules based on the engines field in your package.json.

How about this?

sindresorhus · 2018-01-14T22:58:06Z

Do I have a convenient way to determine where are the AVA test files for the current user?

The list you added works if the user just runs AVA with $ ava, but they could be running it with $ ava foo.js and we wouldn't detect it. Those file patterns might also match users of other test frameworks which doesn't compile like AVA does, so I think this approach is a no-go.

Another approach would be to just run a simple regex for require('ava') or from 'ava'; at the top of the file.

sindresorhus · 2018-01-14T22:59:38Z

That should be solved once mysticatea/eslint-plugin-node@846e677 get released. We could enable the rule then.

👍

pvdlg · 2018-01-14T23:07:56Z

The list you added works if the user just runs AVA with $ ava, but they could be running it with $ ava foo.js and we wouldn't detect it. Those file patterns might also match users of other test frameworks which doesn't compile like AVA does, so I think this approach is a no-go.
Another approach would be to just run a simple regex for require('ava') or from 'ava'; at the top of the file.

Ok make sense. Should implement this way then?

pvdlg · 2018-01-14T23:27:41Z

Actually thinking about the AVA files path issue, I wonder if it's a good idea to use a regex to detect if AVA is imported.
That would prevent users to toggles rules or to use overrides in the case of AVA files.

sindresorhus · 2018-01-14T23:28:31Z

Yeah, let's leave it out for now.

pvdlg · 2018-01-14T23:46:17Z

Yeah, let's leave it out for now.

I'm confused, do you mean leaving the default AVA paths that I set in the PR, or removing them?

Like should I revert a78e02e or keep it?

sindresorhus · 2018-01-20T21:47:59Z

Like should I revert a78e02e or keep it?

Revert, but put it in a separate branch so we can continue it later. It's not a priority now, but I would like to solve it at some point.

…d Node version

pvdlg · 2018-01-20T21:52:16Z

Ok revert is done. I pushed the previous code to the ava-default-ignore branch.

sindresorhus · 2018-01-20T22:44:52Z

This looks excellent. Thanks so much for working on this @pvdlg. I'm really happy about being able to use newer rules without having to manually configure them :)

pvdlg added the enhancement label Jan 7, 2018

pvdlg force-pushed the node-engines-rules branch 2 times, most recently from cb75e3f to c8058c9 Compare January 7, 2018 18:47

sindresorhus requested changes Jan 14, 2018

View reviewed changes

pvdlg force-pushed the node-engines-rules branch from 6091205 to b5c5ebb Compare January 14, 2018 23:04

pvdlg and others added 9 commits January 20, 2018 16:51

Enforce rules based on "engines" field in package.json

f5ee7a8

Set target engines to 8 for default AVA files

4fe1658

Update readme.md

0c4ebb7

Update main.js

254b935

Update options-manager.js

cc9d457

Change highlight wording

8b326eb

Typo

9504d9b

Add unicorn/no-new-buffer and unicorn/prefer-spread only for supporte…

911525c

…d Node version

Revert a78e02e

dccf3d8

pvdlg force-pushed the node-engines-rules branch from 19549e1 to dccf3d8 Compare January 20, 2018 21:51

pvdlg and others added 4 commits January 20, 2018 17:10

Set correct Node version for promise/prefer-await-to-then

89eb2ea

Update plugins.js

5db7cb9

Update readme.md

a443020

Update readme.md

18a2e90

Update package.json

b7f9433

sindresorhus approved these changes Jan 20, 2018

View reviewed changes

sindresorhus merged commit 0d18368 into xojs:master Jan 20, 2018

pvdlg deleted the node-engines-rules branch January 20, 2018 22:45

pvdlg mentioned this pull request Jul 24, 2018

Add node/no-unsupported-features rules and fix unit tests #341

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enforce rules based on "engines" field in package.json #281

Enforce rules based on "engines" field in package.json #281

pvdlg commented Jan 7, 2018 •

edited

sindresorhus commented Jan 8, 2018

pvdlg commented Jan 8, 2018

pvdlg commented Jan 10, 2018

sindresorhus Jan 14, 2018

pvdlg Jan 14, 2018

sindresorhus Jan 14, 2018

pvdlg Jan 14, 2018

sindresorhus Jan 14, 2018

sindresorhus Jan 14, 2018

sindresorhus commented Jan 14, 2018

sindresorhus commented Jan 14, 2018

pvdlg commented Jan 14, 2018

pvdlg commented Jan 14, 2018 •

edited

sindresorhus commented Jan 14, 2018

pvdlg commented Jan 14, 2018 •

edited

sindresorhus commented Jan 20, 2018

pvdlg commented Jan 20, 2018 •

edited

sindresorhus commented Jan 20, 2018

Enforce rules based on "engines" field in package.json #281

Enforce rules based on "engines" field in package.json #281

Conversation

pvdlg commented Jan 7, 2018 • edited

sindresorhus commented Jan 8, 2018

pvdlg commented Jan 8, 2018

pvdlg commented Jan 10, 2018

sindresorhus Jan 14, 2018

Choose a reason for hiding this comment

pvdlg Jan 14, 2018

Choose a reason for hiding this comment

sindresorhus Jan 14, 2018

Choose a reason for hiding this comment

pvdlg Jan 14, 2018

Choose a reason for hiding this comment

sindresorhus Jan 14, 2018

Choose a reason for hiding this comment

sindresorhus Jan 14, 2018

Choose a reason for hiding this comment

sindresorhus commented Jan 14, 2018

sindresorhus commented Jan 14, 2018

pvdlg commented Jan 14, 2018

pvdlg commented Jan 14, 2018 • edited

sindresorhus commented Jan 14, 2018

pvdlg commented Jan 14, 2018 • edited

sindresorhus commented Jan 20, 2018

pvdlg commented Jan 20, 2018 • edited

sindresorhus commented Jan 20, 2018

pvdlg commented Jan 7, 2018 •

edited

pvdlg commented Jan 14, 2018 •

edited

pvdlg commented Jan 14, 2018 •

edited

pvdlg commented Jan 20, 2018 •

edited