Skip to content
This repository has been archived by the owner on Dec 21, 2022. It is now read-only.

🚚 Deprecation von TSLint und Umzug nach ESLint #20

Open
heikomat opened this issue Mar 24, 2019 · 16 comments
Open

🚚 Deprecation von TSLint und Umzug nach ESLint #20

heikomat opened this issue Mar 24, 2019 · 16 comments

Comments

@heikomat
Copy link
Contributor

heikomat commented Mar 24, 2019

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

  1. Bewusstsein für die Notwendigkeit der Umstellung schaffen (Geschieht hiermit).
  2. 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.
  3. 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:

  1. Die notwendigen Pakete mit den Regelsets erstellen.
  2. Entweder den POC verwenden, oder ein minimales Demo-Setup zur Verwendung von ESLint in einem TypeScript Projekt bereitstellen.
  3. ESLint in den ts_monorepo_watch-POC einbauen.
  4. 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
Copy link
Contributor

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
Copy link

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

@heikomat
Copy link
Contributor Author

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

@heikomat
Copy link
Contributor Author

heikomat commented Mar 27, 2019

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
Copy link
Contributor

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
Copy link
Contributor Author

heikomat commented Mar 27, 2019

@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
Copy link
Contributor Author

heikomat commented Mar 27, 2019

@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
Copy link
Contributor Author

@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:

  1. 5Minds ESLint-Regeln auf vorderman bringen (siehe ⬆️ eslint Regelset aktualisieren javascript-guidelines#12)
  2. ein sinnvolles 5Minds ESLint TypeScript Regelset erstellen (angelehnt an unsere TSLint regeln), welches das basis 5Minds ESLint regelset extended
  3. 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
Copy link

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

NullEnt1ty commented Apr 2, 2019

  • 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
Copy link
Contributor Author

heikomat commented Apr 5, 2019

@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
Copy link
Contributor Author

heikomat commented May 1, 2019

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 added a commit that referenced this issue May 2, 2019
**Changes:**
Hier mein Vorschlag für ein eslint-typescript Regelset, wie in #20 besprochen.
- Das Paket ist in version 1.0.0-alpha.5 veröffentlicht.
- Wie es zu installieren und verwenden ist, ist in der README beschrieben

PRs: #21
@heikomat
Copy link
Contributor Author

heikomat commented May 2, 2019

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
Copy link

webia1 commented May 16, 2020

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants