🚚 Deprecation von TSLint und Umzug nach ESLint

heikomat commented 5 years ago

Wie hier und hier zu entnehmen ist, liegt die Zukunft des Typescript-Lintings nicht bei TSLint; Sondern bei ESLint!

ESLint ist sehr viel ausgereifter, performanter und hat mehr Regeln und Einstellungsmöglichkeiten als TSLint. Zudem lassen sich die selben ESLint-Regeln für JavaScript- und TypeScript Projekte anwenden, was die doppelte Pflege von Regeln überflüssig macht.

Deshalb dieses Issue: Als überblick über die aktuelle Situation, und zum Festhalten eines Migrationsplans.

Die aktuelle Situation bei 5Minds

Es gibt für JS-Projekte ein ESLint-Regelset, welches eine Erweiterung des Airbnb-Regelsets ist
Es gibt für TS-Projekte ein TSLint-Regelset, welches komplett selbst geschrieben ist, sich aber an dem ESLint-Regelset orientiert
Es gibt ein TSLint-ESLint-Regelset, welches eine Erweiterung des tslint-Regelsets ist, und dabei versucht dieses mit einem Plugin um die ESLint-Regeln zu erweitern (mit nur mäßigem Erfolg)
Es gibt die @essential-projects/tslint-config, welches auf dem TSLint-ESLint-Regelset basiert, und es minimal abändert
In TypeScript-Projekten wird idr. die @essential-projects/tslint-config verwendet.

Die aktuelle Situation bzgl ESLint

(Die meisten Infos sind von hier)

TSLint wird als deprecated angesehen, und die großen Namen (wie z.B. Microsoft selbst) arbeiten am Umzug zu ESLint.
Um ESLint das linten von TypeScript-Dateien zu erlauben gibt es den @typescript-eslint/parser, welcher ein replacement für den ESLint eigenen parser ist. Der Parser muss dafür nur npm installiert, und in der ESLint-config angegeben werden.
Um zusätzlich zu den ESLint-Regeln TypeScript-spezifische Regeln anwenden zu können, gibt es das @typescript-eslint/eslint-plugin, welches in Kombination mit dem @typescript-eslint/parser arbeitet, und nach und nach die TypeScript spezifischen TSLint-Regeln implementiert. Das Plugin muss dafür nur npm installiert, und in der ESLint-config angegeben werden.
Für die, die nicht auf ihre TSLint-Regeln verzichten können, gibt es das @typescript-eslint/eslint-plugin-tslint, was es einem erlaubt in der ESLint-config eine tslint.json anzugeben, und das plugin geht dann beim ESLint Aufruf her und startet eine TSLint instanz.

Das Ziel

Folgende Ziele wären erstrebenswert:

Es gibt ein ESLint-Regelset, welches sowohl für JS, als auch für TS Projekte verwendet wird.
Es gibt ein ESLint-Regelset, welches ausschließlich TS-spezifische Regeln definiert.
In JS-Projekten wird das ESLint-Regelset ganz normal verwendet.
In TS-Projekten wird @typescript-eslint/parser zum Anwenden der ESLint Regeln verwendet.
In TS-Projekten wird @typescript-eslint/eslint-plugin zum Anwenden der TS-spezifischen ESLint-Regeln verwendet.

Dadurch gewinnen wir folgende Vorteile:

Keine nennenswerten Unterschiede mehr zwischen den TS-Regeln und den JS-Regeln, weil es (bis auf die TypeScript-spezifischen) die selben sind.
Einfachere Wartung der Regeln.
Deutlich besseren community support des verwendeten linters.
(vermutlich) perfomanteres linting.

Der Plan

Bewusstsein für die Notwendigkeit der Umstellung schaffen (Geschieht hiermit).
Einen POC zur Verwendung von ESLint in TS-Projekten bauen.
- Aber nur mit dem Parser und dem Plugin für TS-Spezifische Regeln, NICHT mit dem Plugin welches TSLint startet, da das bestimmt bald obsolet ist.
Mit dem POC evaluieren, ob die Verwendung von ESLint in TypeScript Projekten heute bereits plausibel ist, oder ob das noch ein paar Monate dauert.

Falls Plausibel:

Die notwendigen Pakete mit den Regelsets erstellen.
Entweder den POC verwenden, oder ein minimales Demo-Setup zur Verwendung von ESLint in einem TypeScript Projekt bereitstellen.
ESLint in den ts_monorepo_watch-POC einbauen.
TSLint in den aktiven Projekten durch ESLint ersetzen.

Eure Beteiligung

Ich weiß, dass einigen (mich eingeschlossen) dieses Thema recht wichtig ist, andere sich einfach dafür interessieren, und man alleine sowieso nicht alles berücksichtigen kann. Deshalb würde ich mich über Feedback und Input von allen, die sich davon angesprochen fühlen freuen. Vor Allem aus unterschiedlichen Teams, denn sollte das alles so klappen, wäre ja durchaus mehr als ein Team davon betroffen.

Leider kann man nur 10 Leute Assignen. Deshalb mentione ich hier alle, von denen ich glaube, dass sie sich evtl dafür interessieren könnten. Wer möchte kann sich dann ja selbst assignen: @5ebastianMeier @moellenbeck @ElRaptorus @Vyperus @LeoTT @NullEnt1ty @rrrene @lmoe @cmg-dev @alexanderkasten @MiguelACarballo @Paulomart @S3bastianGriesa @SteffenKn

Falls ich wen vergessen habe, lasst es mich wissen!

NullEnt1ty commented 5 years ago

Danke für die gute Zusammenfassung. Ich glaube, ich habe da nichts zu ergänzen. Wenn der Kurs Richtung "ESLint only" geht, kann ich das nur begrüßen. Ein Regelset ist besser als zwei (oder zumindest ein Regelset + "Addon" für TypeScript).

5ebastianMeier commented 5 years ago

Bin klar dafür unter der Bedingung, dass das Tooling im POC sauber funktioniert.

heikomat commented 5 years ago

Sehr cool! Ich werde heute abend mal einen POC aufsetzen :)

heikomat commented 5 years ago

Der minimal-POC ist jetzt hier zu finden:

Es gab absolut keine Schwierigkeiten beim Aufsetzen, alles hat auf anhieb funktioniert
Es verwendet das 5Minds ESLint Regelset als Basis
Es definiert zusätzlich zwei TypeScript-Spezifische Regeln
Die Integration in VSCode inkl. autofix-on-save funktioniert

NullEnt1ty commented 5 years ago

Funktioniert bei mir ebenfalls ohne Probleme. Sieht gut aus 👍

Interessant wäre zu wissen, ob das in anderen Editoren auch funktioniert. Verwendet hier jemand etwas anderes als VS Code?

heikomat commented 5 years ago

@ElRaptorus

Ist korrekt. Das Regelset in dem PoC ist auch noch nicht repräsentativ, sondern dient nur zur Demonstration, dass das Anwenden von Regeln grundsätzlich erstmal geht.

Das bauen der korrekten Regelsets kommt (zeitnah) in einem späteren Schritt

heikomat commented 5 years ago

@larspapen @NullEnt1ty Import klappt jetzt. Man kann die modul-resoultion für das eslint-import plugin konfigurieren: https://github.com/5minds/typescript_eslint_poc/commit/9bb4b79ef0a04f876fc37b4ac036ff6db8562b3f (siehe https://github.com/benmosher/eslint-plugin-import/tree/master/resolvers/node)

heikomat commented 5 years ago

@5ebastianMeier und ich haben uns heute ein wenig zusammen gesetzt und geprüft, ob es bei der ESLint verwendung irgendwelche dealbreaker geben könnte, und sind zu dem Schluss gekommen, dass das alles recht vielversprechend aussieht:

Man kann scheinbar nicht nur rules extenden, sondern auch alle anderen Eigenschaften (parser, plugins, etc). Das setzt aber voraus, dass im konsumierenden Projekt die entsprechenden dependencies isntalliert sind -> Der TS-Parser und die Plugins die wir verwenden werden, werden also also peerDependencies angegeben werden müssen.
Es ist sehr sinnvoll, eine .eslintignore ins Projekt zu legen, welches z.b. dist und node_modules definiert und so vom linten ausschließen.
ESlint hat bei ersten tests evtl nicht alle Fehler gefunden, die TSLint gefunden hat (weil ESLint halt noch nicht ausreichend konfiguriert war), hat aber jetzt schon teilweise Fehler gefunden, die TSLint nicht gefunden hat.

Die nächsten Schritte sehen deshalb wie folgt aus:

5Minds ESLint-Regeln auf vorderman bringen (siehe https://github.com/5minds/javascript-guidelines/pull/12)
ein sinnvolles 5Minds ESLint TypeScript Regelset erstellen (angelehnt an unsere TSLint regeln), welches das basis 5Minds ESLint regelset extended
Wenn Beide Pakete nach bestem Wissen und Gewissen konfiguriert sind, das TypeScript Regelset in einem Projekt anwenden, und die beiden Regelsets bei bedarf tweaken/feinjustieren

Punkte 1 und 2 sollten bis Ende der Woche machbar sein. Und nach Punkt 3 fehlt auch nurnoch ein kleines Demoprojekt aufzusetzen, was beschreibt wie man das set verwendet (welche Dependencies, Dateien und Konfigurationen man braucht), und dann kann mit der Umsetzung in den Projekten begonnen werden :)

Shinigami92 commented 5 years ago

Ich bin mir nicht sicher ob ich noch etwas überlesen habe, aber ich fänd es super hilfreich wenn am Ende eine ESLint "extends": "tslint:recommended" heraus käme, die einem die Basis der besten Konfigurationen zusammen fasst. Von da aus kann man dann ja gerne weiter je nach wünschen des jeweiligen Projektes tweaken.

heikomat commented 5 years ago

Ich habe angefangen das eslint-config-5minds-typescript Paket zu schreiben. Dabei ist aufgefallen, dass es einige wenige Regeln gibt, die parserServices benötigen. D.h. damit diese Regeln funktionieren, muss man in der .eslintrc.json den key parserOptions.project mitgeben, dessen Wert auf die tsconfig.json des projekts zeigt.

Das funktioniert auch soweit, allerdings ist das extrem langsam. Mit den (übrigens nur 4) Regeln die das benötigen, ist die Linting-Zeit in einem großen Monorepo von 5.5 Sekunden auf 150 Sekunden gestiegen.

Ich habe auch erfahren, dass ein npm Paket mehrere ESLint Regelsets bereitstellen kann. Deshalb habe ich das Paket testweise mal so eingerichtet, dass man entweder 5minds-typescript extenden kann, oder aber 5minds-typescript/fast. Letzteres ist eine erweiterung der 5minds-typescript regeln, bei der die vier Regeln die parserServices benötigen abgeschaltet sind.

Habe das so in der art auch bereits in der (noch unfertigen) readme dokumentiert.

Hier hätte ich gerne mal eure Meinung zu:

Findet ihr die 4 Regeln nicht so wichtig, und der Performancegewinn ist es Wert sie abzuschalten, oder
Ist euch die Linting-Performance egal, und ihr würdet die 4 Regeln drin lassen?

(@ElRaptorus @NullEnt1ty @5ebastianMeier)

heikomat commented 5 years ago

Ich habe das Paket jetzt grade auch mal als 1.0.0-alpha.2 gepublished. Was man alles braucht und wie man es benutzt steht in der Readme.

Die Regeln sind zwar so gut es ging an unsere Regeln und Wünsche angelehnt, aber noch nicht wirklich getestet (nur so ein bisschen angetestet). Deshalb gilt es jetzt hier die Regeln mal testweise in Projekten anzuwenden und alles so anzupassen, wie wir es brauchen.

NullEnt1ty commented 5 years ago

Ist euch die Linting-Performance egal, und ihr würdet die 4 Regeln drin lassen?

Von 5,5 auf 150 Sekunden in deinem Testfall ist schon eine Hausnummer. Da würde ich im Zweifel auf vier Regeln verzichten und sie gegebenenfalls je nach Größe des Projektes speziell für das Projekt aktivieren.

heikomat commented 5 years ago

@NullEnt1ty @ElRaptorus @5ebastianMeier Habe das aktuelle Regelset (5minds-typescript/fast version 1.0.0-alpha.3) grade mal in einem größeren Projekt ausprobiert und ein paar regeln gelockert und getweaked.

Ich würde mich freuen, wenn ihr das Regelset auch mal ausprobieren könntet, und Feedback gebt, was ihr ändern/entfernen/hinzufügen würdet :)

heikomat commented 5 years ago

Da in den letzten ~4 Wochen kein weiteres Feedback gekommen ist, habe ich das Regelset jetzt mal vollständig in einem größeren Projekt angewendet, dabei nochmal 3-4 Regeln getweaked, und den PR für das Regelset geöffnet: #21

Sollten in den nächsten ~2 Wochen keine Änderungsvorschläge kommen, werde ich das Regelset so wie's ist als 1.0.0er releasen.

@ElRaptorus Ich vermute mal, dass ihr bei der Process-Engine (zumindest auf lange Sicht) auch auf eslint umsteigt. Jetzt wäre der optimale Zeitpunkt, das ganze mal auszuprobieren und ggf. noch Anpassungsvorschläge einfließen zu lassen.

// cc @5ebastianMeier @NullEnt1ty

heikomat commented 5 years ago

Da der PR nach kleinen typo-fixes auf Zustimmung gestoßen ist, habe ich das Paket in Version 1.0.0 deployed. Feedback und begründete Änderungsvorschläge sind natürlich weiterhin willkommen :)

webia1 commented 4 years ago

@heikomat Das war bis jetzt die beste Zusammenfassung, die ich gelesen habe, vielen Dank!

5minds / typescript-guidelines