Skip to content

Commit 1a9616a

Browse files
FSVetazskye2k2
andauthored
v6-alpha (#11)
Major release: v6-alpha Consume latest eslint-config-frontier-react. Remove all Polymer-based, Code Climate-related, and legacy configuration and documentation. Upgrade eslint to version 8. Update snapshots to take all new rules into account. Centralize all of our typical overrides (.ts, .stories, .test, etc.), and compartmentalize into its own `es6.js` file. --------- Co-authored-by: Clif Bergmann <[email protected]>
1 parent 67f22ae commit 1a9616a

15 files changed

+2963
-934
lines changed

.eslintrc.js

+1-29
Original file line numberDiff line numberDiff line change
@@ -1,29 +1 @@
1-
// Basic .eslintrc.js file that loads the the frontier shared eslint configuration, and then the extension/override provided by the configuration in index.js just for local demonstration purposes. Also contains example `deprecate` rules.
2-
module.exports = {
3-
extends: [
4-
'eslint-config-frontier',
5-
'eslint-config-standard',
6-
// 'plugin:eslint-plugin-sonarjs/recommended' // Disabled globally, for now, because it is a much higher standard than Tree's existing code currently adheres to. Enable on a case-by-case basis, if you wish.
7-
'plugin:promise/recommended',
8-
'./index.js'
9-
],
10-
plugins: [
11-
// Enable plugins that are not natively supported by Code Climate. Otherwise results in build errors.
12-
'eslint-plugin-bestpractices',
13-
'eslint-plugin-deprecate',
14-
'eslint-plugin-promise',
15-
'eslint-plugin-sonarjs',
16-
'eslint-plugin-test-selectors' // NOTE: Only runs against JSX
17-
],
18-
rules: {
19-
'deprecate/function': ['error',
20-
{'name': 'deprecatedFunction', 'use': 'function x from package y'}
21-
],
22-
'deprecate/import': ['error',
23-
{'name': 'path/to/legacyModule', 'use': 'module x'}
24-
],
25-
'deprecate/member-expression': ['error',
26-
{'name': '$.each', 'use': 'native forEach'}
27-
]
28-
}
29-
};
1+
module.exports = { extends: ['./index.js'], settings: { jest: { version: 29 } } }

.gitignore

-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,5 @@
99
.vscode/*
1010
**/node_modules
1111
/reports/**
12-
analysis.json
1312
npm-debug.log
1413
/demo/test/snapshots/local*

.npmrc

+1
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
package-lock=false
2+
save=true

.nvmrc

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
16

README.md

+34-50
Original file line numberDiff line numberDiff line change
@@ -8,27 +8,30 @@ This central configuration is a potential breaking point for _all_ of our code i
88

99
**Process:**
1010

11+
1. Run `npm test` (to determine if any significant rules have changed since the last release)
12+
- The tests will likely fail. Verify newly-consumed rules against the current [snapshot](/demo/test/snapshots/linting-config.test.js.md) file.
13+
1. After verifying, run `npm run test:update`.
1114
1. Make dependency/configuration updates.
12-
1. Run `npm test`.
15+
1. Run `npm test` (to determine new changes in linting results or configuration).
1316
- The tests should likely fail. Verify your expectations against the current [snapshot](/demo/test/snapshots/linting-config.test.js.md) file.
1417
1. After you have your results how you want them, run `npm run test:update`.
1518
- The tests should now pass.
16-
1. If you want see how your changes would impact a codebase, you can either `npm link` or copy+paste the contents of `local-linting-final-config.json` temporarily into the target `.eslintrc` file.
1719

18-
> TODO: Update the documentation below to be current, and not include things like Code Climate
20+
<!--1. If you want see how your changes would impact a codebase, you can either `npm link` or copy+paste the contents of `local-linting-final-config.json` temporarily into the target `.eslintrc` file.
21+
-->
1922

2023
Why extra rules? Because we believe in linting, and we have become converted to the additional rules enforced by the following plugins:
2124

2225
- [eslint-plugin-bestpractices](https://github.com/skye2k2/eslint-plugin-bestpractices)
2326
- [eslint-plugin-deprecate](https://github.com/AlexMost/eslint-plugin-deprecate)
2427
- [eslint-plugin-html](https://github.com/BenoitZugmeyer/eslint-plugin-html)
28+
- [eslint-plugin-import](https://github.com/import-js/eslint-plugin-import) (implemented by Frontier)
2529
- [eslint-plugin-jsdoc](https://github.com/gajus/eslint-plugin-jsdoc)
26-
- [eslint-plugin-json](https://github.com/azeemba/eslint-plugin-json)
30+
- [eslint-plugin-json](https://github.com/azeemba/eslint-plugin-json) (adopted by Frontier)
2731
- [eslint-plugin-promise](https://github.com/xjamundx/eslint-plugin-promise)
2832
- [eslint-plugin-sonarjs](https://github.com/SonarSource/eslint-plugin-sonarjs)
29-
- [eslint-config-standard](https://github.com/standard/eslint-config-standard)
3033

31-
> POTENTIALLY WORTH CONSIDERING IN THE FUTURE (MAY NOT WORK BECAUSE OF NEEDING SOMETHING LIKE BABEL?):
34+
> POTENTIALLY WORTH CONSIDERING IN THE FUTURE (MAY NOT WORK BECAUSE OF NEEDING SOMETHING EXTRA?):
3235
3336
> - 'eslint-plugin-i18next' // SEEMS LIKE TOO MANY FALSE POSITIVES
3437
> - 'eslint-plugin-json-format' // DOESN'T SEEM TO WORK
@@ -39,51 +42,17 @@ Why extra rules? Because we believe in linting, and we have become converted to
3942

4043
## Usage:
4144

42-
1. Add either `eslint-config-frontier` or `eslint-config-frontier-react` as a devDependency.
43-
4445
1. Add this repository as a package devDependency:
4546

46-
> "eslint-config-tree": "github:fs-webdev/eslint-config-tree#semver:^4",
47+
> "eslint-config-tree": "github:fs-webdev/eslint-config-tree#semver:^6",
4748
48-
1. In your `eslintrc.js` file, put the following:
49+
1. Add an `eslintrc.js` file, with the following:
4950
<pre><code>module.exports = {
5051
extends: [
51-
'eslint-config-frontier', // or '@fs/eslint-config-frontier-react'
5252
'eslint-config-tree'
53-
],
54-
plugins: [
55-
'eslint-plugin-bestpractices',
56-
'eslint-plugin-deprecate',
57-
'eslint-plugin-promise',
58-
'eslint-plugin-sonarjs',
59-
'eslint-plugin-test-selectors'
60-
]
61-
}</code></pre>
62-
63-
1. Add a `.codeclimate.eslintrc.js`
64-
<pre><code>module.exports = {
65-
extends: [
66-
'./eslint-config-frontier.js', // or '@fs/eslint-config-frontier-react'
67-
'./eslint-config-tree.js'
6853
]
6954
}</code></pre>
7055

71-
1. Add both `tree` and the frontier eslint configuration of your choice as Code Climate `prepare` resources (see: [extended eslint docs](https://www.familysearch.org/frontier/legacy/ui-components/eslint-config-frontier/)).
72-
73-
1. Set this simplified eslint configuration as the chosen config in your Code Climate's `plugins`.
74-
<pre><code>plugins:
75-
eslint:
76-
enabled: true
77-
channel: "eslint-6"
78-
config:
79-
config: .codeclimate.eslintrc.js
80-
extensions:
81-
- .html
82-
- .js
83-
- .json
84-
ignore_warnings: true
85-
</code></pre>
86-
8756
1. Enjoy.
8857

8958
## HOWTOs:
@@ -126,7 +95,7 @@ Utilize a file linting config modifier like so:
12695
12796
```
12897

129-
Note that `--` comments are permitted and a good idea to include.
98+
Note that `--` comments are permitted and a very good idea to include.
13099

131100
<!--
132101
DOES NOT CURRENTLY WORK, AND bestpractices/no-eslint-disable SHOULD PROBABLY BE MODIFIED TO TAKE THIS INTO ACCOUNT.
@@ -139,17 +108,13 @@ Or disable BOTH the desired rule and the no-eslint-disable rule:
139108

140109
### How to deal with `Definition for rule '{RULE}' was not found.` errors:
141110

142-
This is a known state when submitting a new file to Code Climate for the first time, since they do not support all of the linting extensions we wish to use. If you are seeing these warnings when linting locally, you may have `eslint` installed globally, but not the additional dependency. We do not recommend running `eslint` globally for this reason (see: https://github.com/eslint/eslint/issues/6732). All Tree repositories should include all dependencies required to be able to run `eslint` locally in their respective directories.
111+
If you are seeing these warnings when linting locally, you may have `eslint` installed globally, but not the additional dependency. We do not recommend running `eslint` globally for this reason (see: https://github.com/eslint/eslint/issues/6732). All Tree repositories should include all dependencies required to be able to run `eslint` locally in their respective directories.
143112

144113
If you have recently updated dependencies and see this error locally, then there is a possibility that your editor's linting integration is out-of-sync that can be resolved by restarting your editor.
145114

146115
### How to not have tons of `jsdoc` warnings:
147116

148-
The `jsdoc` warnings are only triggered for functions that have an jsdoc extended comment block (`/** */`) directly above the function declaration. Omit this, or just use a short comment (`//`) or a standard extended comment (`/* */`) to keep from applying `jsdoc` rules to functions not requiring fastidious documentation. Or follow all of the rules.
149-
150-
### How to do even trickier things with linting configuration:
151-
152-
Just read the manual: https://eslint.org/docs/7.0.0/user-guide/configuring
117+
The `jsdoc` warnings are only triggered for functions that have an jsdoc extended comment block (`/** */`) directly above the function declaration. Omit this, add an extra space, or just use a short comment (`//`) or a standard extended comment (`/* */`) to keep from applying `jsdoc` rules to functions not requiring fastidious documentation. Or follow all of the rules.
153118

154119
<details>
155120
<summary>Maintenance Notes</summary>
@@ -162,14 +127,25 @@ If there has been a change (say you added a new rule, or there is a new valid vi
162127

163128
## Notes
164129

130+
- Why no lockfile? Because we (currently) trust our dependencies, and do not want to constantly have to be verifying and manually releasing new versions of this convenience configuration. We may decide to be more precise in the future.
165131
- As noted in the `Testing/Updating` section, the only validation we do is to run linting against a file with a set of known failures. So we make sure to run `npm test` via a pre-push hook, and releases are automatically performed by a GitHub webhook.
166-
- Because this is a public repository, there are complications in adding references to private services and communications channels, so there is no Travis CI build and no Code Climate integration.
132+
- Because this is a public repository, there are complications in adding references to private services and communications channels, so there is no Travis CI build.
167133
- Coverage reporting ends up reporting on `lint-output.js`, instead of `index.js`, which is unhelpful, and so is also not used, for now.
168134

169135
</details>
170136

171137
## Changelog:
172138

139+
<details>
140+
<summary>Version 6 - ESLint 8</summary>
141+
142+
- Update all linting subdependencies. Remove redundant plugins (eslint-plugin-json adopted by Frontier).
143+
- Remove Code Climate/Polymer-related configurations and documentation.
144+
- Add new final configuration test.
145+
- Inherit more configuration from frontier (finally).
146+
147+
</details>
148+
173149
<details>
174150
<summary>Version 5 </summary>
175151

@@ -200,3 +176,11 @@ If there has been a change (say you added a new rule, or there is a new valid vi
200176
- ESLint and dependencies based on version 6.
201177

202178
</details>
179+
180+
<details>
181+
<summary>Version 1 - ESLint 5</summary>
182+
183+
- ESLint and dependencies based on version 5.
184+
- Add eslint-plugin-bestpractices, eslint-plugin-deprecate, eslint-plugin-html, eslint-plugin-jsdoc, eslint-plugin-json, eslint-plugin-promise, eslint-plugin-sonarjs, eslint-config-standard
185+
186+
</details>

demo/.eslintrc.js

+6-5
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
// Example directory override (still has access to all configuration, extensions, and plugins at the root level, but overrides the settings for this directory and below, if no other .eslintrc.js file exists)
2-
// Common uses are to loosen our normally tight linting ruleset for code that is under development so that TODOs and the like do not crowd out more important warnings (provided that such overrides are removed upon production release)
31
module.exports = {
42
rules: {
53
'bestpractices/no-eslint-disable': 'error',
6-
'sonarjs/no-duplicated-branches': 'error'
7-
}
8-
};
4+
'sonarjs/no-duplicated-branches': 'error',
5+
'deprecate/function': ['error', { name: 'deprecatedFunction', use: 'function x from package y' }],
6+
'deprecate/import': ['error', { name: 'path/to/legacyModule', use: 'module x' }],
7+
'deprecate/member-expression': ['error', { name: '$.each', use: 'native forEach' }],
8+
},
9+
}

demo/example.js

+48-47
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
/* eslint no-console: "off" -- node scripts use the console, so disable for the whole file */
44

55
/*
6-
* Since developers have the ability to disable linting in-line, we keep track of the times where this is done, because if done irresponsibly, this is a significant code smell.
7-
*/
6+
* Since developers have the ability to disable linting in-line, we keep track of the times where this is done, because if done irresponsibly, this is a significant code smell.
7+
*/
88
// eslint-disable-next
99

1010
// fixMe: Actually make this work
@@ -16,9 +16,9 @@
1616
* As long as you separate the comment block from the declaration, JSDOC rules will not be applied. But the comment will still show through many IDE's definition hot-linking.
1717
*/
1818

19-
function functionWithoutJSDocWarningsBecauseTheSectionWasCompletelyExcluded () {
20-
console.log('ASHLDKFJHASKFJSDHFKJSDHFKLSDJHFLJKSDHFLKSDJFHKSDLJFHLSDKJF');
21-
return true;
19+
function functionWithoutJSDocWarningsBecauseTheSectionWasCompletelyExcluded() {
20+
console.log('ASHLDKFJHASKFJSDHFKJSDHFKLSDJHFLJKSDHFLKSDJFHKSDLJFHLSDKJF')
21+
return true
2222
}
2323

2424
/**
@@ -28,93 +28,94 @@ function functionWithoutJSDocWarningsBecauseTheSectionWasCompletelyExcluded () {
2828
* @param b
2929
* @returns
3030
*/
31-
function ONE_FUNCTION_TO_BRING_THEM_ALL_AND_IN_THE_DARKNESS_BIND_THEM (params) {
32-
}
31+
function ONE_FUNCTION_TO_BRING_THEM_ALL_AND_IN_THE_DARKNESS_BIND_THEM(params) {}
3332

34-
const myPromise = new Promise();
33+
const myPromise = new Promise()
3534

3635
/**
3736
* @note
3837
*/
3938
myPromise.then((a) => {
4039
if (true === false) {
41-
return Promise.resolve();
40+
return Promise.resolve()
4241
} else {
43-
forgotToDefine();
42+
forgotToDefine()
4443
}
45-
});
44+
})
4645

47-
const variable = (true) ? true : true;
46+
const variable = true ? true : true
4847

49-
if (window === undefined && window === undefined && params === true) {
50-
ONE_FUNCTION_TO_BRING_THEM_ALL_AND_IN_THE_DARKNESS_BIND_THEM('a', 'b');
51-
const deprecatedImport = require('path/to/legacyModule');
52-
deprecatedImport.execute();
53-
deprecatedFunction();
54-
shortFunction();
55-
$.each();
56-
return;
57-
debugger;// Make sure we get in here to set the value correctly
48+
export function ReturnEarly() {
49+
if (window === undefined && window === undefined && params === true) {
50+
ONE_FUNCTION_TO_BRING_THEM_ALL_AND_IN_THE_DARKNESS_BIND_THEM('a', 'b')
51+
const deprecatedImport = require('path/to/legacyModule')
52+
deprecatedImport.execute()
53+
deprecatedFunction()
54+
shortFunction()
55+
$.each()
56+
return
57+
debugger // Make sure we get in here to set the value correctly
58+
}
5859
}
5960

6061
/* We value sonarjs rules enough to test them, here. Sorry for the mess. */
6162

62-
function shortFunction (arg) {
63+
function shortFunction(arg) {
6364
if (arg) {
64-
console.log(true);
65+
console.log(true)
6566
}
66-
return true;
67+
return true
6768
}
6869

6970
// sonarjs/no-identical-functions checks function bodies of three lines and above
70-
function duplicateFunction (arg) {
71+
function duplicateFunction(arg) {
7172
if (arg) {
72-
console.log(true);
73+
console.log(true)
7374
}
74-
return true;
75+
return true
7576
}
7677

7778
// sonarjs/no-all-duplicated-branches
7879
if (true) {
79-
shortFunction();
80+
shortFunction()
8081
} else {
81-
shortFunction();
82+
shortFunction()
8283
}
8384

8485
// sonarjs/max-switch-cases, sonarjs/no-duplicated-branches
8586
switch (1) {
8687
case 1:
87-
break;
88+
break
8889
case 2:
8990
switch (2) {
9091
case 1:
91-
duplicateFunction();
92-
duplicateFunction();
93-
break;
92+
duplicateFunction()
93+
duplicateFunction()
94+
break
9495
case 1:
95-
duplicateFunction();
96-
duplicateFunction();
97-
break;
96+
duplicateFunction()
97+
duplicateFunction()
98+
break
9899
}
99-
break;
100+
break
100101
case 3:
101-
break;
102+
break
102103
case 4:
103-
break;
104+
break
104105
case 5:
105-
break;
106+
break
106107
case 6:
107-
break;
108+
break
108109
case 7:
109-
break;
110+
break
110111
case 8:
111-
break;
112+
break
112113
case 9:
113-
break;
114+
break
114115
case 10:
115-
break;
116+
break
116117
case 11:
117-
break;
118+
break
118119
default:
119-
break;
120+
break
120121
}

0 commit comments

Comments
 (0)