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

Scripts: Improve recommended settings included in the package #17027

Merged
merged 5 commits into from
Aug 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ module.exports = {
root: true,
extends: [
'plugin:@wordpress/eslint-plugin/recommended',
'plugin:jest/recommended',
'plugin:eslint-comments/recommended',
],
plugins: [
Expand All @@ -30,6 +29,8 @@ module.exports = {
wp: 'off',
},
rules: {
'@wordpress/dependency-group': 'error',
'@wordpress/gutenberg-phase': 'error',
'@wordpress/react-no-unsafe-timeout': 'error',
'no-restricted-syntax': [
'error',
Expand Down Expand Up @@ -123,16 +124,19 @@ module.exports = {
'**/@(benchmark|test|__tests__)/**/*.js',
],
},
{
files: [
'packages/jest*/**/*.js',
],
extends: [
'plugin:@wordpress/eslint-plugin/test-unit',
],
},
{
files: [ 'packages/e2e-test*/**/*.js' ],
env: {
browser: true,
},
globals: {
browser: 'readonly',
page: 'readonly',
wp: 'readonly',
},
extends: [
'plugin:@wordpress/eslint-plugin/test-e2e',
],
},
],
};
8 changes: 5 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@
"enzyme": "3.9.0",
"eslint-plugin-eslint-comments": "3.1.2",
"eslint-plugin-import": "2.18.2",
"eslint-plugin-jest": "22.14.1",
"fast-glob": "2.2.7",
"fbjs": "0.8.17",
"fs-extra": "8.0.1",
Expand Down
6 changes: 6 additions & 0 deletions packages/babel-preset-default/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## Master

### Bug Fixes

- Added missing `@wordpress/element` dependency which is used internally.

## 4.4.0 (2019-08-05)

### Bug Fixes
Expand Down
4 changes: 1 addition & 3 deletions packages/babel-preset-default/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,9 @@
"@babel/runtime": "^7.4.4",
"@wordpress/babel-plugin-import-jsx-pragma": "file:../babel-plugin-import-jsx-pragma",
"@wordpress/browserslist-config": "file:../browserslist-config",
"@wordpress/element": "file:../element",
"core-js": "^3.1.4"
},
"peerDependencies": {
"@babel/core": "^7.0.0"
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So does this change essentially mean that packages consuming wp-script don't need @babel/core as a dependency anymore?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point, it was added as a regular dependencies so it is redundant here.

"publishConfig": {
"access": "public"
}
Expand Down
10 changes: 10 additions & 0 deletions packages/block-serialization-spec-parser/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"overrides": [
{
"files": [ "shared-tests.js" ],
"extends": [
"plugin:@wordpress/eslint-plugin/test-unit"
]
}
]
}
7 changes: 7 additions & 0 deletions packages/eslint-plugin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,18 @@

- The [`@wordpress/no-unused-vars-before-return` rule](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/no-unused-vars-before-return.md) has been improved to exempt object destructuring only if destructuring to more than one property.
- Stricter JSDoc linting using [`eslint-plugin-jsdoc`](https://github.com/gajus/eslint-plugin-jsdoc).
- Stricter validation enabled for test files only using new `test-e2e` and `test-unit` rulesets.

### New Features

- New Rule: [`@wordpress/no-unguarded-get-range-at`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/no-unguarded-get-range-at.md)
- Enable `wp` global by default in the `recommended` config.
- New ruleset `test-e2e` added for end-to-end tests validation.
- New ruleset `test-unit` added for unit tests validation.

### Enhancements

- Remove `@wordpress/dependency-group` and `@wordpress/gutenberg-phase` rules from the `custom` and `recommended` configs and leave them as opt-in features.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 This is mostly a breaking change for consumers that are using these rules. Most consumers won't want/need them so I like.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it should be highlighted as breaking change? I wasn’t sure where to put it. Given that, there are other breaking changes, we can more freely move it if necessary 😃

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure either. The scope of when it's considered a breaking change is fairly narrow and it's only breaking in the sense the rules won't be applied automatically anymore.


## 2.4.0 (2019-08-05)

Expand Down
5 changes: 5 additions & 0 deletions packages/eslint-plugin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,15 @@ The `recommended` preset will include rules governing an ES2015+ environment, an

Alternatively, you can opt-in to only the more granular rulesets offered by the plugin. These include:

- `custom`
- `es5`
- `esnext`
- `jsdoc`
- `jshint` (legacy)
- `jsx-a11y`
- `react`
- `test-e2e`
- `test-unit`

For example, if your project does not use React, you could consider extending including only the ESNext rules in your project using the following `extends` definition:

Expand Down
2 changes: 0 additions & 2 deletions packages/eslint-plugin/configs/custom.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ module.exports = {
'@wordpress',
],
rules: {
'@wordpress/dependency-group': 'error',
'@wordpress/gutenberg-phase': 'error',
'@wordpress/no-unused-vars-before-return': 'error',
'@wordpress/valid-sprintf': 'error',
'@wordpress/no-base-control-with-label-without-id': 'error',
Expand Down
22 changes: 22 additions & 0 deletions packages/eslint-plugin/configs/recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,26 @@ module.exports = {
document: true,
wp: 'readonly',
},
overrides: [
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ntwb, I was referring to this changes in #17061 (comment). I linked wrong PR 😃

{
// Unit test files and their helpers only.
files: [
'**/@(test|__tests__)/**/*.js',
'**/?(*.)test.js',
],
extends: [
require.resolve( './test-unit.js' ),
],
},
{
// End-to-end test files and their helpers only.
files: [
'**/specs/**/*.js',
'**/?(*.)spec.js',
],
extends: [
require.resolve( './test-e2e.js' ),
],
},
],
};
13 changes: 13 additions & 0 deletions packages/eslint-plugin/configs/test-e2e.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
module.exports = {
extends: [
'plugin:jest/recommended',
],
env: {
browser: true,
},
globals: {
browser: 'readonly',
page: 'readonly',
wp: 'readonly',
},
};
5 changes: 5 additions & 0 deletions packages/eslint-plugin/configs/test-unit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module.exports = {
extends: [
'plugin:jest/recommended',
],
};
1 change: 1 addition & 0 deletions packages/eslint-plugin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"main": "index.js",
"dependencies": {
"babel-eslint": "^10.0.2",
"eslint-plugin-jest": "^22.15.1",
"eslint-plugin-jsdoc": "^15.8.0",
"eslint-plugin-jsx-a11y": "^6.2.3",
"eslint-plugin-react": "^7.14.3",
Expand Down
14 changes: 12 additions & 2 deletions packages/is-shallow-equal/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,17 @@
{
"root": true,
"extends": "plugin:@wordpress/eslint-plugin/es5",
"extends": [
"plugin:@wordpress/eslint-plugin/es5"
],
"env": {
"node": true
}
},
"overrides": [
{
"files": [ "@(test|benchmark)/*.js" ],
"extends": [
"plugin:@wordpress/eslint-plugin/recommended"
]
}
]
}
4 changes: 0 additions & 4 deletions packages/is-shallow-equal/benchmark/.eslintrc.json

This file was deleted.

4 changes: 0 additions & 4 deletions packages/is-shallow-equal/test/.eslintrc.json

This file was deleted.

1 change: 1 addition & 0 deletions packages/scripts/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
- The bundled `puppeteer` dependency has been updated from requiring `1.6.1` to requiring `^1.19.0` ([#16875](https://github.com/WordPress/gutenberg/pull/16875)). It uses Chromium v77 instead of Chromium v69.
- The bundled `jest-puppeteer` dependency has been updated from requiring `^4.0.0` to requiring `^4.3.0` ([#16875](https://github.com/WordPress/gutenberg/pull/16875)).
- The bundled `eslint` dependency has been updated from requiring `^5.16.0` to requiring `^6.1.0`.
- The bundled `@wordpress/eslint-plugin` dependency has been updated to the next major version `^3.0.0` due to new ESLint rules enabled for all test files.

## 3.4.0 (2019-08-05)

Expand Down