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

Remove TSLint (and optionally replace with eslint-plugin) #240

Closed
SleeplessByte opened this issue Apr 4, 2019 · 6 comments · Fixed by #276
Closed

Remove TSLint (and optionally replace with eslint-plugin) #240

SleeplessByte opened this issue Apr 4, 2019 · 6 comments · Fixed by #276
Assignees
Labels
chore 🔧 Meta related task such as build, test, linting, maintainers.json etc.

Comments

@SleeplessByte
Copy link
Member

As you might have heard, TSLint has been officially deprecated and will reach EOL soon.

The official replacement is a plugin on top of eslint:

This track should remove tslint dependency moving forward and optionally replace it with the eslint-plugin version. That said:

  • It is a ground rule on exercism that we teach fluency, not stylistic choices. Auto formatting such as prettier and linting such as tslint should only be applied to help with the fluency of the language (capture potential mistake) and not force a particular style guide if there is no official one.
  • tslint was not an official standard by Microsoft, but rather a company called Palantir. There are various other linters, such as xo which have a completely different default ruleset.
  • Because there is no official standard, all stylistic rules should be removed from recommendation. Instead, only apply rules that help with language features and bugs. For eslint this is eslint:recommended.

As nice as prettier is, its rules are no official standard and should therefore not be applied.

@masters3d
Copy link
Contributor

I am cool with replacing TSLint as long as we move the configuration to ESLint. Just removing would be a regression.

@SleeplessByte
Copy link
Member Author

Regression in what sense?

I think that having linting on for maintainers is amazing and we should do! But the exercise level should probably have very loose rules for the students. Currently it's enforcing style which is not official and there are competing style guides, which is when forcing style is against exercism policy :)

For example: this forces no semi-colon and indent with spaces. Whilst I wholly agree with them, these are company/project policies and should not be in there for the student.

@masters3d
Copy link
Contributor

Yeah. We could edit the lint script to use a different configuration or like you said we could remove it. I don’t see why we would since it is not something that gets automatically ran like pretty or similar auto formatters.

"scripts": {
"test": "tsc --noEmit -p . && jest --no-cache",
"lint": "tsc --noEmit -p . && tslint ".ts?(x)"",
"lintci": "tslint "
.ts?(x)" --force"
},

@SleeplessByte
Copy link
Member Author

SleeplessByte commented Apr 4, 2019

Ah, that is a common mistake:

I know that we don't (red: run it automatically), but that's not how IDEs work. VS Code for example will pick up the eslint configuration by default if the plugin is active. Atom en Webstorm too!

The issue here is that having .eslintrc or tslint.config in the directory is enough for many IDEs to auto run its rules.

The steps we have taken at javascript so far are:

  • Replace track-level eslint to recommended (from strict airbnb-base)
  • Turn off a few stylistic rules that were left
  • Turn on a few grammar / bug / language feature rules that are not on by default
  • Change the ci that it uses airbnb-base when running instead of recommended

Which would be very similar to what you're suggesting, so I approve 👍

Edit: clarification: my recommendation would be to replace it with the new plugin, not remove it, but also to downgrade the "severity of strictness" for the track level, and make the change you're suggesting which would use the project level config instead.

@masters3d
Copy link
Contributor

The way I see the progression is to move to eslint while maintaining the same config and not style changes. Figure out a way to have the default be different than what gets ran on CI which should match the original.

@SleeplessByte SleeplessByte added the chore 🔧 Meta related task such as build, test, linting, maintainers.json etc. label Apr 12, 2019
@SleeplessByte SleeplessByte self-assigned this May 31, 2019
@SleeplessByte
Copy link
Member Author

Working on this.

ErikSchierboom referenced this issue in ErikSchierboom/typescript Jan 27, 2021
* Add implementing a concept docs

* add missing files in example file structure and change existing to .ts

* add .gitignore in exercise folder

* add stubs for concept files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore 🔧 Meta related task such as build, test, linting, maintainers.json etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants