5minds / javascript-guidelines

Our customized eslint config inspired by airbnb
0 stars 0 forks source link

⬆️ eslint Regelset aktualisieren #12

Closed heikomat closed 5 years ago

heikomat commented 5 years ago

Dieser PR soll die eslint-regeln auf einen aktuelle(re)n Stand bringen.

Wir machen zwar nicht mehr viel mit JS, sondern arbeiten hauptsächlich mit TS, doch das dient als Vorbereitung, um anschließend unsere tslint-regeln auf vordermann zu bringen.

Das wiederum dient als Vorarbeit unsere tslint-eslint-regeln aufzuräumen, was dann zum schluss wenn alles fertig ist https://github.com/5minds/typescript-guidelines/issues/18 fixen soll.

Da seit dem letzten Update sehr viele Regeln hinzu gekommen sind, konnte ich mir nicht zu allen Änderungen weitgehende Gedanken machen. Habe aber dennoch zu allen neuen Regeln kurz beschrieben was sie machen, und bei Änderungsempfehlungen meinerseits diese dazu geschrieben.

Zudem habe ich neue eslint-Regeln, die bei AirBnB aber deaktiviert sind nicht gelistet.

Änderungen

Fixes #8, #10 https://github.com/5minds/JS.Foundation/issues/109

Neue Regeln in dem AirBnB-Regelset

Best practices - `no-restricted-properties` untersagt die Verwendung von bestimmten eigenschaften an bestimmten Objekten, z.B. `global.isFinite` (besser: `Numer.isFinite`) und `arguments.callee` (ist deprecated, man sollte named functions benutzen). - `no-return-await` verbietet `return await` an stellen, an denen das `await` keinen Unterschied macht. - `no-useless-catch` Untersagt try/catch-blöcke, wenn in dem catch-block nichts weiter passiert als das rethrowen des Fehlers. - `no-useless-return` Untersagt das verwenden von `return;` wenn nach dem Return kein Code mehr kommt, der damit übersprungen werden könnte. - `prefer-promise-reject-errors` Untersagt das rejecten von Promises mit Objekten die kein `Error` sind. die AirBnB regeln erlauben allerdings weiterhin das rejecten mit undefined (= `Promise.reject()`). - `require-await` Diese Regel verbietet es, das async-keyword zu verwenden, wenn in der zugehörigen Funktion kein `await` verwendet wird. **Die AirBnB guidelines DEAKTIVIEREN diese Regel**. Grund dafür ist vermutlich der, dass man mit `async` erzwingen kann, dass die Funktion ein Promise zurück gibt, auch wenn sie eig. synchron ist. Siehe [When Not To Use It](https://eslint.org/docs/rules/require-await#when-not-to-use-it) für mehr Infos. **Empfehlung:** Ich würde diese Regel wieder aktivieren. Das entspricht unseren tslint-regeln, und wer dennoch unbedingt ein `async` ohne `await` verwenden will, muss die regel für die entsprechende Funktion halt per kommentar deaktivieren. - `require-unicode-regexp` Erzwingt die Verwendung des `u`-flags bei Regexen, damit UTF-16 Character korrekt behandelt werden (z.b. `/aaa/u` anstatt `/aaa/`).**Die AirBnB guidelines DEAKTIVIEREN diese Regel** (@cmg-dev).
Errors - `for-direction` Verhindert, dass bei for-loops in die falsche Richtung iteriert wird, was zu einer Endlosschleife führen würde. - `getter-return` Erzwingt, dass eine propert-getter-funktion ein `return` hat. - `no-async-promise-executor` Verbietet die Verwendung von async-functions beim erstellen eines neuen Promise. (`new Promise((resolve, reject) => {...})` ist damit erlaubt, `new Promise(async (resolve, reject) => {...})` aber nicht! ). Grund dafür ist, dass Fehler des inneren Promise verloren gehen könnten, und das äußere Promise idr. entweder nicht notwendig ist, und die Funktion ohne das äußere Promise vereinfacht werden könnte. - `no-await-in-loop` Verbietet die verwendung von `await` in loops, da sie ein Anzeichen für eine liste von Promises sein können, die parallel abgehandelt werden könnten, aber nacheinander abgehandelt werden. **Ich finde diese Regel sollte nicht aktiviert werden!** Bengründung: Wenn die Ergebnisse der durchläuft nicht unabhängig voneinander sind, benötigt man das `await` in dem loop. - `no-compare-neg-zero` Verbietet es, mit `-0` zu vergleichen (z.b. `x === -0`), denn das evaluiert zu `true` sowohl für `x = 0` als auch für `x = -0`. - `no-unsafe-negation` Verbietet Ausdrücke wie `if (!key in object)` weil mit großer Wahrscheinlichkeit `if (!(key in object))` gemeint ist - `require-atomic-updates` siehe [require-atomic-updates](https://eslint.org/docs/rules/require-atomic-updates#disallow-assignments-that-can-lead-to-race-conditions-due-to-usage-of-await-or-yield-require-atomic-updates). **Die AirBnBguidelines DEAKTIVIEREN diese Regel**
ES6 (vorher in es2017.js) - `arrow-body-style` Die regel war vorher bereits aktiv, aber AirBnB hat hier die Option `requireReturnForObjectLiteral` abgeschaltet. Das macht aber für uns keinen Unterschied, da wir die Regel eh mit `[error, always]` überschreiben. - `no-duplicate-imports` Verbietet mehrere import-statements für das selbe Modul. **Die Regel wird hier deaktiviert, weil sie durch das `eslint-plugin-import` ersetzt wird** - `prefer-destructuring` Erzwingt das verwenden von `const {foo} = someObject` anstatt `const foo = someObject.foo`. **Empfehlung:** Ich würde die Regel bei uns nicht aktivieren. Destructuring kann elegant sein, aber auch unübersichtlich werden, und manchmal ist ein direkter zugriff einfach einfacher zu lesen. Deshalb würde ich destructuring erlauben, aber nicht erzwingen. - `symbol-description` Erzwingt beim erstellen eines `Symbol()` das angeben einer Beschreibung (z.B. `const test = Symbol('someDescription')`).
Imports - `import/order` Erzwingt nun die Gruppierung der Imports nach `'builtin', 'external', 'internal'` - `import/max-dependencies` Verbietet es, aus mehr als 10 Modulen zu importieren (aka max. 10 import-statements pro Datei) - `import/no-absolute-path` Verbietet imports mit einem absoluten Pfad (aka imports die mit einem `/` anfangen), weil diese an die Ordnerstruktur des Computers gebunden sind, auf dem der import hinzugefügt wurde - `import/no-dynamic-require` Verbietet dynamische require-stastements wie `require(someVariable)`. Stattdessen muss das zu importierende Modul direkt angegeben werden (z.b. `require('someModule')`). - `import/no-webpack-loader-syntax` Verbietet webpack-loader-syntax (z.b. `require("my-loader!./my-awesome-module")`). Webpack loader sollten stattdessen in der webpack configuration file definiert werden. - `import/no-named-default` Verbietet `import { default as foo, bar } from './foo.js';` und erzwingt stattdessen `import foo, { bar } from './foo.js';` - `import/no-self-import` Verbietet, dass ein Modul sich selbst importiert - `import/no-cycle` Verbietet zirkuläre imports - `import/no-useless-path-segments` Verbietet suboptimale Pfadangaben in imports wie z.B. `./test/../foo`.
Node - `no-buffer-constructor` Verbietet die Verwendung von `new Buffer(something)`. Der Buffer constructor ist deprecated und man sollte stattdessen `Buffer.from()`, `Buffer.alloc()` und `Buffer.allocUnsafe()` verwenden.
Strict Hier hat sich nichts geändert
Style - `function-paren-newline` Steht auf `consistent` und erzwingt dass die Umbrüche innerhalb eines Parametersatzes konsistent sind. Wenn also ein Parameter in einem Parametersatz eine newline hat, müssen alle eine haben und umgekehrt. - `implicit-arrow-linebreak` Erzwingt, dass der body einer arrow-function mit implizitem return immer neben dem Pfeil ist, nicht darunter. **Empfehlung:** Da wir mit der `arrow-body-style` Regel implizite Returns eh verboten haben, sollten wir diese Regel deaktivieren. - `lines-between-class-members` Erzwingt eine Leerzeile zwischen Klassenmembern (außer über dem ersten und unter dem letzten) - `lines-around-directive` (Ist deprecated) Erzwingt eine Leerzeile vor und nach Direktiven wie `"use strict"` - `no-multi-assign` Verbietet multi-assignments wie `const a = b = c = 5;` - `no-plusplus` Verbietet `foo++`. Stattdessen soll `foo += 1` verwendet werden **Diskussionswürdig!** - `no-restricted-syntax` War vorher bereits aktiv, wurde aber jetzt im das Verbot von `for ... of` loops erweitert. **Empfehlung:** Wir sollten `for...of` loops erlauben. Sie sind die sauberste und lesbarste variante über etwas zu iterieren, sind überall bei uns im Einsatz und erlauben mit async-iterators das konsumieren von ReadableStreams. - `no-tabs` Verbietet den Tab-character vollständig - `nonblock-statement-body-position` Erzwingt eine einheitliche Platzierung des bodys von blöcken ohne geschweifte klammern (z.b. bei einzeiler-ifs). **Empfehlung:** Da wir blöcke ohne geschweifte Klammern eh nicht erlauben, sollten wir diese Regel deaktivieren - `object-curly-newline` Erzwingt, dass ab einer gewissen Anzahl von Properties in einem Objekt oder imports in einem import statement diese untereinander, statt nebeneinander geschrieben werden. AirBnB erzwingt die newlines ab dem vierten Element. - `operator-linebreak` Erzwingt einen einheitlichen stil bei mehrzeiligen Operationen. Beispielsweise erzwingt die Regel, so wie so von AirBnB konfiguriert ist, dass die operatoren am Anfang stehen: ```JavaScript const fullHeight = borderTop + innerHeight + borderBottom; ```
Variables Hier hat sich nichts geändert