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

Switch to eslint + prettier #5870

Merged
merged 14 commits into from
Jun 28, 2023
Merged

Switch to eslint + prettier #5870

merged 14 commits into from
Jun 28, 2023

Conversation

dibarbet
Copy link
Member

@dibarbet dibarbet commented Jun 27, 2023

TSLint is deprecated and we're lacking a consistent style throughout the repo

This PR adds eslint + typescript eslint + prettier with the generally recommended settings or with settings that were previously configured by TSLint.

Also importantly changes the case of files to be camelCase. Interfaces are currently ignored - those need to be renamed to not be IInterface. The typical ts way is to just name them as standard types with regular names.

There are likely more rules we can turn on, but I would do those in a followup PR.

I think commit at a time is the best way to look at this.

ab9ed4a look at the .eslintrc.js file for the config and skim the changes to see if anything looks out of place
57f79a1 finishes addressing the errors introduced by the initial config in the commit above.
f7296da adds file name rules to eslint. The rest is basically just file renames and can probably be skimmed.
033b0aa removes TSLint and adds the remaining tslint rules to the eslint config + adds prettier config rules.
01ab588 is the result of running the automatic prettier fixer. Look at the previous commit for the rules.
0798590 fixes a couple more outstanding issues
And then the rest are more small fixes that got left out / mixed in the first set of changes.

NOTE

This introduces a new warning when running the build, specifically

=============

WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree.

You may find that it works just fine, or you may not.

SUPPORTED TYPESCRIPT VERSIONS: >=3.3.1 <5.1.0

YOUR TYPESCRIPT VERSION: 5.1.3

Please only submit bug reports when using the officially supported version.

=============

This is a known issue and most likely not a problem for us, see typescript-eslint/typescript-eslint#6934
We should update when there is a version with the fix.

Copy link
Member

@JoeRobich JoeRobich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love this work.

.prettierrc Show resolved Hide resolved
src/json.ts Outdated Show resolved Hide resolved
@dibarbet dibarbet marked this pull request as ready for review June 27, 2023 22:34
@dibarbet dibarbet requested review from a team as code owners June 27, 2023 22:34
@dibarbet
Copy link
Member Author

@davidwengier / @allisonchou mind taking a look from the Razor side?
and @WardenGnaw for the debugger side?

Would like to get this in since it affects almost every file in the repo.

@dibarbet dibarbet merged commit 95bb3ac into dotnet:main Jun 28, 2023
@dibarbet dibarbet deleted the eslint branch June 28, 2023 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants