Support typescript-eslint when it becomes available

[johnnyreilly]

TypeScript are planning to move to using eslint in future to lint TypeScript: microsoft/TypeScript#29288

See also: https://eslint.org/blog/2019/01/future-typescript-eslint

Given this plugin supports running tslint side by side with the compiler it makes sense to allow the same functionality with eslint when that becomes ready. It will likely involve integrating with this project:

https://github.com/typescript-eslint/typescript-eslint

Given typescript-eslint requires a fairly high version to work we will only be able to support this with newer versions of TypeScript; but that's fine.

If someone wants to look at integrating this into the plugin when typescript-eslint is sufficiently mature that would be amazing! ❤️

https://medium.com/palantir/tslint-in-2019-1a144c2317a9

[johnnyreilly]

This PR that adds support for typescript-eslint to create-react-app could well prove useful as inspiration when someone is looking into adding eslint support directly to fork-ts-checker-webpack-plugin

facebook/create-react-app#6513

😀🌻

[ux-engineer]

As a new comer with this package, I'm wondering what's the current state of ESLint support?

TypeScript's future is with ESLint instead of TSLint (as linked above). README doesn't even mention ESLint so it seems I'd need to wait before I can give this a try. Although linting integration is optional, I'm not sure if the package would be fully useful without it.

[johnnyreilly]

As yet it hasn't been added - eslint TypeScript support is still a very new thing. If someone would like to submit a PR adding support we'd be super grateful.

In the longer term given that tslint is being deprecated, I'd imagine support for tslint with the plugin will be too. But that's going to be some time away.

The main purpose of the plugin is type checking. Linting comes a close second. Until support is added to the plugin there's nothing stopping you doing what create-react-app are doing currently; that is to say running eslint alongside the fork-ts-checker-webpack-plugin.

[johnnyreilly]

A Plan!

Yesterday I took the functionality in place in create-react-app for a spin. Based upon that experience I think that TypeScript eslint has got to a pretty good place.

I'm not sure whether I'm going to have time to attempt this in the near future, but I think implementation of this functionality may actually prove fairly straightforward. I've certainly an idea of a rough plan - if someone wants to take have a crack at this and report back that'd be amazing! Here's the plan:

1. Implement a new eslint option in https://github.com/Realytics/fork-ts-checker-webpack-plugin/blob/master/src/ApiIncrementalChecker.ts - note this is the API incremental checker which is the default unless you're using Vue. We'd want to share this functionality between that and the IncrementalChecker in future, but let's at least get started 😁

2. Look at the eslint-loader for prior art. Note: this is exactly what create-react-app is using as it was already rolling with eslint long before TypeScript was even on the horizon. The code of interest is here: https://github.com/webpack-contrib/eslint-loader/blob/master/index.js

There's actually very little work to do I think - most of what we need will be achieved by using something like the following:

var engine = new eslint.CLIEngine(config);
lint(engine, input, resourcePath);

// where input is the source file contents
// and resourcePath is name of file

function lint(engine, input, resourcePath) {
  return engine.executeOnText(input, resourcePath, true);
}

Our work is pretty much just mapping that back into a NormalizedMessage and that's it.. tick! 😆

If someone wants to have a crack at this I'd be happy to collaborate.

cc @piotr-oles @phryneas

[johnnyreilly]

I've tentatively started the beginnings on work on this in the linked PR. Very preliminary stages right now, people should feel free to pitch in and help as I predict this maybe slightly stop / start from my side 😄

[zackschuster]

could tslint support be kept as a plugin somehow? i have no plans to migrate as eslint is 4x the install size before typescript support & doesn't provide any extra value for me.

[johnnyreilly]

Since TSLint is deprecated and will certainly become incrementally less useful and useable over time, I think it's safe to say that we're likely to remove it. Older versions of fork-ts-checker-webpack-plugin will always be available which will support it however.

That said, support won't be removed in the near future. So rest easy for now.

[johnnyreilly]

Shipped with 1.4.0

https://github.com/TypeStrong/fork-ts-checker-webpack-plugin/releases/tag/v1.4.0

[johnnyreilly]

See how to use this here: https://johnnyreilly.com/typescript-and-eslint-meet-fork-ts-checker-webpack-plugin

[stavalfi]

@johnnyreilly Thats such a great addition to this plugin! Thanks!!

But there is a problem - https://www.npmjs.com/package/eslint-import-resolver-webpack does not work any more.. when using eslint-loader, someone (i guess it's eslint-loader) calls the webpack config function and get the resovle details. overall, there are alot of calls to webpack config functions.

now, after removing eslint-loader in favor of this feature, there is no call to webpack config function. Thats may be a clue for what is wrong..

the actual errors from eslint are that all the aliases I defined in webpack.resolve are not recognized by https://www.npmjs.com/package/eslint-import-resolver-webpack :(

[johnnyreilly]

@stavalfi I'm afraid I don't understand what you mean... What doesn't work?

[akillingbeck89]

Hmm unsure if this is working, I'm on "fork-ts-checker-webpack-plugin": "^1.4.3"

Previously, when using tslint, I would get the log output:

Starting type checking and linting service...

Now I only get,

Starting type checking service

I tested it by changing some code which fails linting, and then running and everything compiles successfully (eslint is definitely not being run).

This is the section of my webpack which uses the forkTs plugin:
new ForkTsCheckerWebpackPlugin({ async: false, watch: paths.appSrc, eslint: true, eslintOptions: { extensions: ['.js','.ts','.tsx'] }, tsconfig: paths.appTsConfig })

[johnnyreilly]

The logging should be fixed - we're just not checking for ESLint as well as TSLint correctly here:

fork-ts-checker-webpack-plugin/src/index.ts

Line 602 in 34b16c5

this.tslint

If you'd like to submit a PR I'll review. Arguably we should also log whether we're linting with TSLint or ESLint too

If there's an issue please can you replicate the issue using the example here: https://github.com/TypeStrong/ts-loader/tree/master/examples/fork-ts-checker-webpack-plugin

And raise a separate issue against the plugin with a reproducible repo? Thanks.

[theandrewlane]

I'm facing the same issue as @akillingbeck89 :(