Remove tslint in favour of eslint #834

ematipico · 2019-04-18T10:37:55Z

Do you want to request a feature or report a bug?
Feature, better a chore.

What is the current behavior?
At the moment we use two linters to lint the projects

If the current behavior is a bug, please provide the steps to reproduce.
N/A

If this is a feature request, what is motivation or use case for changing the behavior?
Using eslint to lint all the files.

Main changes that there are in this PR:

created a super generic .eslintrc.js file. Which we already have and I removed all the the rules regarding eslint-plugin-node.
inside the cli folder I created a new eslint config file that inherits from the main one and it adds all the node rules that have been removed from the main config file
inside each package I crated an eslint file that takes care of using the proper typescript parser plus and plugin. I also added the prettier configuration so it won't collide with formatting
tests should be always inside a folder so we can have a specific eslint config file for those: by default typescript-eslint wants for every function to return the type (which is good), but when we define the tests, we always have describe("", () => {}) which is annoying. So better disable the rule for test files.

pranshuchittora · 2019-04-18T11:44:58Z

IMO both ESLint & TSLint have some voids. Pls read this

ematipico · 2019-04-18T12:18:16Z

Yeah I know but it's difficult to maintain two linters with two different way of configuring things. Now that eslint supports TypeScript and core members of TypeScript are helping @typescript-eslint, I'd say it's a good thing to move to just one to help and make maintainability easier.

anshumanv · 2019-04-18T13:44:10Z

In order to avoid bifurcating the linting tool space for TypeScript, we therefore plan to deprecate TSLint and focus our efforts instead on improving ESLint’s TypeScript support.

Positive change IMO. ✨

ematipico · 2019-04-18T13:44:29Z

TypeScript interfaces are a bit messed up and it's full of duplicates. (We have two NodePath which risk to collide).

After this PR we should think about doing the following things:

remove our types about JSCodeshift and use @types/jscodeshift
remove our yeoman types and use @types/yeoman-generator

- fixed typescript types - removed tslint commands - fixed tests

webpack-bot · 2019-04-18T14:16:21Z

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

evenstensberg

lgtm, nice, thought the same. I guess the file changes for func is because of linting?

evenstensberg · 2019-04-18T14:50:30Z

.eslintrc.js

-		"node": true,
-		"es6": true,
-		"jest": true
+	// "root": true,


remove unneeded comments?

evenstensberg · 2019-04-18T14:51:27Z

packages/generate-loader/index.ts

 import LoaderGenerator from "@webpack-cli/generators/loader-generator";
 import * as yeoman from "yeoman-environment";

-import { IYeoman } from "./types/Yeoman";


Why is type removed?

Because it was unused

ematipico · 2019-04-18T14:58:55Z

Yes changes to the functions were entirely to the types, most of them it was to apply the cast. I don't why it was building honestly, there were a number of typescript issues. Anyway, they are all fixed

anshumanv

Looks good overall, cool

anshumanv · 2019-04-19T02:38:14Z

packages/utils/package-manager.ts

 	return require("global-modules");
 }
+
+export function spawnChild(pkg: string): SpawnSyncReturns<Buffer> {


Docstrings for this are left above.

anshumanv · 2019-04-19T02:39:16Z

packages/utils/validate-identifier.ts

 // are only applied when a character is found to actually have a
 // code point above 128.

 /* tslint:disable: max-line-length */


Can be removed.

anshumanv · 2019-04-19T02:39:26Z

packages/utils/validate-identifier.ts

 let nonASCIIidentifierChars =
-"\u200c\u200d\xb7\u0300-\u036f\u0387\u0483-\u0487\u0591-\u05bd\u05bf\u05c1\u05c2\u05c4\u05c5\u05c7\u0610-\u061a\u064b-\u0669\u0670\u06d6-\u06dc\u06df-\u06e4\u06e7\u06e8\u06ea-\u06ed\u06f0-\u06f9\u0711\u0730-\u074a\u07a6-\u07b0\u07c0-\u07c9\u07eb-\u07f3\u0816-\u0819\u081b-\u0823\u0825-\u0827\u0829-\u082d\u0859-\u085b\u08e4-\u0903\u093a-\u093c\u093e-\u094f\u0951-\u0957\u0962\u0963\u0966-\u096f\u0981-\u0983\u09bc\u09be-\u09c4\u09c7\u09c8\u09cb-\u09cd\u09d7\u09e2\u09e3\u09e6-\u09ef\u0a01-\u0a03\u0a3c\u0a3e-\u0a42\u0a47\u0a48\u0a4b-\u0a4d\u0a51\u0a66-\u0a71\u0a75\u0a81-\u0a83\u0abc\u0abe-\u0ac5\u0ac7-\u0ac9\u0acb-\u0acd\u0ae2\u0ae3\u0ae6-\u0aef\u0b01-\u0b03\u0b3c\u0b3e-\u0b44\u0b47\u0b48\u0b4b-\u0b4d\u0b56\u0b57\u0b62\u0b63\u0b66-\u0b6f\u0b82\u0bbe-\u0bc2\u0bc6-\u0bc8\u0bca-\u0bcd\u0bd7\u0be6-\u0bef\u0c00-\u0c03\u0c3e-\u0c44\u0c46-\u0c48\u0c4a-\u0c4d\u0c55\u0c56\u0c62\u0c63\u0c66-\u0c6f\u0c81-\u0c83\u0cbc\u0cbe-\u0cc4\u0cc6-\u0cc8\u0cca-\u0ccd\u0cd5\u0cd6\u0ce2\u0ce3\u0ce6-\u0cef\u0d01-\u0d03\u0d3e-\u0d44\u0d46-\u0d48\u0d4a-\u0d4d\u0d57\u0d62\u0d63\u0d66-\u0d6f\u0d82\u0d83\u0dca\u0dcf-\u0dd4\u0dd6\u0dd8-\u0ddf\u0de6-\u0def\u0df2\u0df3\u0e31\u0e34-\u0e3a\u0e47-\u0e4e\u0e50-\u0e59\u0eb1\u0eb4-\u0eb9\u0ebb\u0ebc\u0ec8-\u0ecd\u0ed0-\u0ed9\u0f18\u0f19\u0f20-\u0f29\u0f35\u0f37\u0f39\u0f3e\u0f3f\u0f71-\u0f84\u0f86\u0f87\u0f8d-\u0f97\u0f99-\u0fbc\u0fc6\u102b-\u103e\u1040-\u1049\u1056-\u1059\u105e-\u1060\u1062-\u1064\u1067-\u106d\u1071-\u1074\u1082-\u108d\u108f-\u109d\u135d-\u135f\u1369-\u1371\u1712-\u1714\u1732-\u1734\u1752\u1753\u1772\u1773\u17b4-\u17d3\u17dd\u17e0-\u17e9\u180b-\u180d\u1810-\u1819\u18a9\u1920-\u192b\u1930-\u193b\u1946-\u194f\u19b0-\u19c0\u19c8\u19c9\u19d0-\u19da\u1a17-\u1a1b\u1a55-\u1a5e\u1a60-\u1a7c\u1a7f-\u1a89\u1a90-\u1a99\u1ab0-\u1abd\u1b00-\u1b04\u1b34-\u1b44\u1b50-\u1b59\u1b6b-\u1b73\u1b80-\u1b82\u1ba1-\u1bad\u1bb0-\u1bb9\u1be6-\u1bf3\u1c24-\u1c37\u1c40-\u1c49\u1c50-\u1c59\u1cd0-\u1cd2\u1cd4-\u1ce8\u1ced\u1cf2-\u1cf4\u1cf8\u1cf9\u1dc0-\u1df5\u1dfc-\u1dff\u203f\u2040\u2054\u20d0-\u20dc\u20e1\u20e5-\u20f0\u2cef-\u2cf1\u2d7f\u2de0-\u2dff\u302a-\u302f\u3099\u309a\ua620-\ua629\ua66f\ua674-\ua67d\ua69f\ua6f0\ua6f1\ua802\ua806\ua80b\ua823-\ua827\ua880\ua881\ua8b4-\ua8c4\ua8d0-\ua8d9\ua8e0-\ua8f1\ua900-\ua909\ua926-\ua92d\ua947-\ua953\ua980-\ua983\ua9b3-\ua9c0\ua9d0-\ua9d9\ua9e5\ua9f0-\ua9f9\uaa29-\uaa36\uaa43\uaa4c\uaa4d\uaa50-\uaa59\uaa7b-\uaa7d\uaab0\uaab2-\uaab4\uaab7\uaab8\uaabe\uaabf\uaac1\uaaeb-\uaaef\uaaf5\uaaf6\uabe3-\uabea\uabec\uabed\uabf0-\uabf9\ufb1e\ufe00-\ufe0f\ufe20-\ufe2d\ufe33\ufe34\ufe4d-\ufe4f\uff10-\uff19\uff3f";
+	"\u200c\u200d\xb7\u0300-\u036f\u0387\u0483-\u0487\u0591-\u05bd\u05bf\u05c1\u05c2\u05c4\u05c5\u05c7\u0610-\u061a\u064b-\u0669\u0670\u06d6-\u06dc\u06df-\u06e4\u06e7\u06e8\u06ea-\u06ed\u06f0-\u06f9\u0711\u0730-\u074a\u07a6-\u07b0\u07c0-\u07c9\u07eb-\u07f3\u0816-\u0819\u081b-\u0823\u0825-\u0827\u0829-\u082d\u0859-\u085b\u08e4-\u0903\u093a-\u093c\u093e-\u094f\u0951-\u0957\u0962\u0963\u0966-\u096f\u0981-\u0983\u09bc\u09be-\u09c4\u09c7\u09c8\u09cb-\u09cd\u09d7\u09e2\u09e3\u09e6-\u09ef\u0a01-\u0a03\u0a3c\u0a3e-\u0a42\u0a47\u0a48\u0a4b-\u0a4d\u0a51\u0a66-\u0a71\u0a75\u0a81-\u0a83\u0abc\u0abe-\u0ac5\u0ac7-\u0ac9\u0acb-\u0acd\u0ae2\u0ae3\u0ae6-\u0aef\u0b01-\u0b03\u0b3c\u0b3e-\u0b44\u0b47\u0b48\u0b4b-\u0b4d\u0b56\u0b57\u0b62\u0b63\u0b66-\u0b6f\u0b82\u0bbe-\u0bc2\u0bc6-\u0bc8\u0bca-\u0bcd\u0bd7\u0be6-\u0bef\u0c00-\u0c03\u0c3e-\u0c44\u0c46-\u0c48\u0c4a-\u0c4d\u0c55\u0c56\u0c62\u0c63\u0c66-\u0c6f\u0c81-\u0c83\u0cbc\u0cbe-\u0cc4\u0cc6-\u0cc8\u0cca-\u0ccd\u0cd5\u0cd6\u0ce2\u0ce3\u0ce6-\u0cef\u0d01-\u0d03\u0d3e-\u0d44\u0d46-\u0d48\u0d4a-\u0d4d\u0d57\u0d62\u0d63\u0d66-\u0d6f\u0d82\u0d83\u0dca\u0dcf-\u0dd4\u0dd6\u0dd8-\u0ddf\u0de6-\u0def\u0df2\u0df3\u0e31\u0e34-\u0e3a\u0e47-\u0e4e\u0e50-\u0e59\u0eb1\u0eb4-\u0eb9\u0ebb\u0ebc\u0ec8-\u0ecd\u0ed0-\u0ed9\u0f18\u0f19\u0f20-\u0f29\u0f35\u0f37\u0f39\u0f3e\u0f3f\u0f71-\u0f84\u0f86\u0f87\u0f8d-\u0f97\u0f99-\u0fbc\u0fc6\u102b-\u103e\u1040-\u1049\u1056-\u1059\u105e-\u1060\u1062-\u1064\u1067-\u106d\u1071-\u1074\u1082-\u108d\u108f-\u109d\u135d-\u135f\u1369-\u1371\u1712-\u1714\u1732-\u1734\u1752\u1753\u1772\u1773\u17b4-\u17d3\u17dd\u17e0-\u17e9\u180b-\u180d\u1810-\u1819\u18a9\u1920-\u192b\u1930-\u193b\u1946-\u194f\u19b0-\u19c0\u19c8\u19c9\u19d0-\u19da\u1a17-\u1a1b\u1a55-\u1a5e\u1a60-\u1a7c\u1a7f-\u1a89\u1a90-\u1a99\u1ab0-\u1abd\u1b00-\u1b04\u1b34-\u1b44\u1b50-\u1b59\u1b6b-\u1b73\u1b80-\u1b82\u1ba1-\u1bad\u1bb0-\u1bb9\u1be6-\u1bf3\u1c24-\u1c37\u1c40-\u1c49\u1c50-\u1c59\u1cd0-\u1cd2\u1cd4-\u1ce8\u1ced\u1cf2-\u1cf4\u1cf8\u1cf9\u1dc0-\u1df5\u1dfc-\u1dff\u203f\u2040\u2054\u20d0-\u20dc\u20e1\u20e5-\u20f0\u2cef-\u2cf1\u2d7f\u2de0-\u2dff\u302a-\u302f\u3099\u309a\ua620-\ua629\ua66f\ua674-\ua67d\ua69f\ua6f0\ua6f1\ua802\ua806\ua80b\ua823-\ua827\ua880\ua881\ua8b4-\ua8c4\ua8d0-\ua8d9\ua8e0-\ua8f1\ua900-\ua909\ua926-\ua92d\ua947-\ua953\ua980-\ua983\ua9b3-\ua9c0\ua9d0-\ua9d9\ua9e5\ua9f0-\ua9f9\uaa29-\uaa36\uaa43\uaa4c\uaa4d\uaa50-\uaa59\uaa7b-\uaa7d\uaab0\uaab2-\uaab4\uaab7\uaab8\uaabe\uaabf\uaac1\uaaeb-\uaaef\uaaf5\uaaf6\uabe3-\uabea\uabec\uabed\uabf0-\uabf9\ufb1e\ufe00-\ufe0f\ufe20-\ufe2d\ufe33\ufe34\ufe4d-\ufe4f\uff10-\uff19\uff3f";
 /* tslint:enable: max-line-length */


anshumanv · 2019-04-19T02:42:21Z

.gitignore


 # source maps of docs
 docs/**/*.map
+junit.xml


webpack-bot · 2019-04-20T08:02:56Z

@ematipico Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@anshumanv Please review the new changes.

webpack-bot added the PR: unreviewed label Apr 18, 2019

ematipico changed the title ~~chore: created eslint configs in each package~~ [WIP] Remove tslint of favour of eslint Apr 18, 2019

ematipico changed the title ~~[WIP] Remove tslint of favour of eslint~~ [WIP] Remove tslint in favour of eslint Apr 18, 2019

webpack-bot added the PR: CI-not-ok label Apr 18, 2019

ematipico changed the title ~~[WIP] Remove tslint in favour of eslint~~ Remove tslint in favour of eslint Apr 18, 2019

chore: created eslint configs in each package

ebb136c

- fixed typescript types - removed tslint commands - fixed tests

webpack-bot added PR: CI-ok and removed PR: CI-not-ok labels Apr 18, 2019

evenstensberg approved these changes Apr 18, 2019

View reviewed changes

webpack-bot added PR: reviewed-approved and removed PR: unreviewed labels Apr 18, 2019

chore: removed comment

bf384f9

webpack-bot added PR: review-outdated and removed PR: reviewed-approved labels Apr 18, 2019

anshumanv approved these changes Apr 19, 2019

View reviewed changes

webpack-bot added PR: reviewed-approved and removed PR: review-outdated labels Apr 19, 2019

chore: removed lines that belong to tslint

3200757

webpack-bot added PR: review-outdated and removed PR: reviewed-approved labels Apr 20, 2019

dhruvdutt approved these changes Apr 20, 2019

View reviewed changes

webpack-bot added PR: reviewed-approved and removed PR: review-outdated labels Apr 20, 2019

ematipico merged commit 06b4929 into webpack:master Apr 20, 2019

masi mentioned this pull request May 2, 2019

enable eslinting of .vue files symfony/webpack-encore#504

Closed

ohana54 mentioned this pull request Oct 2, 2019

[BUG]: Error Cannot create property 'context' on string 'Error generating AST' #1064

Closed

Uh oh!

Remove tslint in favour of eslint #834

Remove tslint in favour of eslint #834

Uh oh!

Conversation

ematipico commented Apr 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pranshuchittora commented Apr 18, 2019

Uh oh!

ematipico commented Apr 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anshumanv commented Apr 18, 2019

Uh oh!

ematipico commented Apr 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webpack-bot commented Apr 18, 2019

Uh oh!

evenstensberg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ematipico commented Apr 18, 2019

Uh oh!

anshumanv left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

webpack-bot commented Apr 20, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ematipico commented Apr 18, 2019 •

edited

Loading

ematipico commented Apr 18, 2019 •

edited

Loading

ematipico commented Apr 18, 2019 •

edited

Loading