add eslint typescript support #305

johnnyreilly · 2019-06-30T17:14:05Z

This PR is intended to start the journey towards adding eslint TypeScript support: #203

I'm not sure how long it will take to do this as I'm somewhat time poor right now, so all help is appreciated ❤️ 😄

So, what's happened so far? This:

Disabled tslint --fix during commits - in future we should consider using eslint --fix, but I'd rather do that in a subsequent PR to make this one less noisy.
Added a new test project called project_eslint. This contains some simple eslint errors which are visible in VS Code with the ESLint VS Code plugin.
Added an eslint option to the plugin and put the mechanism in place in service.ts to call it.
Added a basic implementation to both IncrementalChecker (for Vue users) and ApiIncrementalChecker.
Added a first test
Tested it on a simple project. All looks good!

You can see what usage of this looks like by looking at this PR: TypeStrong/ts-loader#960

The plan to deprecate tslint support and remove it in future still stands. However, before we get to that point I'd like to ship eslint support and ensure it's doing what we need.

Having looked at the code around tslint support, it looks like removing tslint support will be a nice simplification of the codebase.

phryneas · 2019-07-02T06:27:14Z

Heyo, I'll try to take a look at this at the weekend - I'm very preoccupied right now. But I'm watching :)

johnnyreilly · 2019-07-02T07:46:43Z

Cool - I'll see if I can take it for a test drive on a real codebase in the meantime. That said, busy week, might have to wait - just wanted to give a heads up that it was happening 😁

Oh side question:

I noticed that rpc.js triggers some errors in the console in vs code sometimes. It doesn't seem to stop tests working but it did get me wondering:

Could / should this file be TypeScript instead of js?
worker-rpc has received updates - should we update fork-ts-checker-webpack-plugin to accommodate at some point?

https://github.com/Realytics/fork-ts-checker-webpack-plugin/blob/master/test/integration/helpers/rpc.js

README.md

johnnyreilly · 2019-07-06T13:35:28Z

Tested this on a large codebase. It works fine, however it's subject to the performance issues that typescript-eslint seems to have; see here: typescript-eslint/typescript-eslint#389

johnnyreilly · 2019-07-07T04:55:37Z

Would someone be able to review this PR please? I think we're ready to go!

phryneas · 2019-07-07T08:23:02Z

Skimmed it so far, looks good but I might have some suggestions.
I'll try to set aside some time this evening for a thorough review.

johnnyreilly · 2019-07-07T15:12:34Z

The promise of a review made me do some of the mini refactorings I hadn't yet got round to 😁

phryneas · 2019-07-07T19:51:57Z

Yeah, sorry, I'm quite tired today and not really up for the task. I commented what I could see right of the bat, not everything might be correct and most of it definitely debatable. I'll give it a second look tomorrow.

Props for the "share eslint between checkers" commit, I was definitely going to suggest that after having read it this morning :)

johnnyreilly · 2019-07-07T20:05:44Z

Yeah, sorry, I'm quite tired today and not really up for the task. I commented what I could see right of the bat, not everything might be correct and most of it definitely debatable. I'll give it a second look tomorrow.

Dude, take your time! I appreciate you looking at all. There's no rush; nothing is on fire 😁

Props for the "share eslint between checkers" commit, I was definitely going to suggest that after having read it this morning :)

Yeah, my DRY-sense was tingling there; had to deal with it. Went slightly further in my last commit to share the logic for using ESLint as well. Felt better about myself after 😉

piotr-oles

Good job! Just a few comments from me :)

src/ApiIncrementalChecker.ts

src/FilesRegister.ts

src/IncrementalChecker.ts

src/makeEslinter.ts

johnnyreilly · 2019-07-08T10:02:55Z

Thanks for the review @piotr-oles - I've made changes 😄

phryneas

So apparently my comments from yesterday were held back by me not "finishing" my review (argh).

So here's what I had written yesterday, I'll come check around in the next few days with a more thorough review.

.gitignore

README.md

package.json

src/IncrementalChecker.ts

src/service.ts

johnnyreilly · 2019-07-09T04:28:33Z

Thanks for the review @phryneas - I've made changes in line with your suggestions

johnnyreilly · 2019-07-10T20:17:54Z

I've just fixed up the merge conflict; so I think this means I technically need another approval 😄

johnnyreilly · 2019-07-11T16:29:20Z

Hey @piotr-oles / @phryneas,

Are either of you good to approve this PR? I'd like to ship and make sure our release infrastructure is still good now we've moved

piotr-oles

I just did another round as there are a lot of changes :)

package.json

src/ApiIncrementalChecker.ts

test/integration/incrementalApi.spec.ts

test/fixtures/project_eslint/src/lib/func.ts

johnnyreilly · 2019-07-11T20:46:29Z

I've addressed the review comments!

piotr-oles · 2019-07-11T20:48:20Z

What about CancellationToken? :)

johnnyreilly · 2019-07-11T20:55:50Z

I replied!

johnnyreilly · 2019-07-11T21:05:58Z

Fixed!

piotr-oles

Great!

johnnyreilly · 2019-07-12T04:24:09Z

Okay - merged.... Let's see if we get a new version published!

piotr-oles · 2019-07-12T05:15:03Z

🎉 This PR is included in version 1.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

johnnyreilly · 2019-07-12T05:21:25Z

Amazing!

johnnyreilly · 2019-07-13T09:48:51Z

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

test: create test fixture for eslint + activate vscode plugin

20d1be2

johnnyreilly mentioned this pull request Jun 30, 2019

Work in progress: add eslint typescript support #304

Closed

johnnyreilly added 5 commits June 30, 2019 19:02

feat: add eslint option infrastructure

6621ec4

feat: add basic implementation (untested) - might work!

014f466

chore: disable eslint in Travis for now

f13f6f3

test: added first test of eslint message being published

6eed41e

test: add error test for eslint

68d4463

johnnyreilly requested review from piotr-oles and phryneas July 1, 2019 22:06

johnnyreilly added 2 commits July 2, 2019 21:28

feat: add eslint support to IncrementalChecker

725ccdb

test: remove duplicate test and up test timeout

f8b1784

johnnyreilly changed the title ~~Work in progress: add eslint typescript support~~ add eslint typescript support Jul 3, 2019

docs: add eslint details to readme

18699eb

johnnyreilly commented Jul 3, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

johnnyreilly mentioned this pull request Jul 3, 2019

migrate fork-ts-checker-webpack-plugin example to use ESLint TypeStrong/ts-loader#960

Merged

johnnyreilly added 3 commits July 3, 2019 22:51

feat: validate eslint and allow options to be passed

072c0e8

fix: correctly output version

f336064

feat: persist eslinter instance between runs

38bc8c9

johnnyreilly added 2 commits July 7, 2019 14:15

feat: incrementalchecker no longer recreates eslint each time

7e55814

feat: share eslint between checkers

3c747fc

feat: share eslint logic between checkers

534c773

piotr-oles requested changes Jul 8, 2019

View reviewed changes

fix: respond to @piotr-oles PR feedback

32ad926

fix: delete stale lints first

a0a14ca

johnnyreilly requested a review from piotr-oles July 8, 2019 20:08

phryneas reviewed Jul 8, 2019

View reviewed changes

.gitignore Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

package.json Show resolved Hide resolved

src/IncrementalChecker.ts Show resolved Hide resolved

src/service.ts Outdated Show resolved Hide resolved

fix: addressed comments suggested by @phryneas

93d4381

piotr-oles previously approved these changes Jul 9, 2019

View reviewed changes

Merge branch 'master' into master

590557b

johnnyreilly dismissed piotr-oles’s stale review via 590557b July 10, 2019 20:16

docs: more eslint info in the readme

48f281c

piotr-oles requested changes Jul 11, 2019

View reviewed changes

package.json Show resolved Hide resolved

src/ApiIncrementalChecker.ts Outdated Show resolved Hide resolved

test/integration/incrementalApi.spec.ts Outdated Show resolved Hide resolved

test/fixtures/project_eslint/src/lib/func.ts Show resolved Hide resolved

johnnyreilly added 2 commits July 11, 2019 21:43

fix: respond to review comments from piotr

605fa17

Merge branch 'master' into master

e32fea7

fix: move cancellation token check

66511f5

piotr-oles approved these changes Jul 11, 2019

View reviewed changes

johnnyreilly merged commit 544f205 into TypeStrong:master Jul 12, 2019

piotr-oles added the released label Jul 12, 2019

phryneas mentioned this pull request Nov 25, 2019

WIP: typescript sys proxy #231

Closed

snyk-bot mentioned this pull request Dec 5, 2019

[Snyk] Upgrade fork-ts-checker-webpack-plugin from 1.1.1 to 1.6.0 dyna-dot/create-react-app#2

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add eslint typescript support #305

add eslint typescript support #305

johnnyreilly commented Jun 30, 2019 •

edited

Loading

phryneas commented Jul 2, 2019

johnnyreilly commented Jul 2, 2019 •

edited

Loading

johnnyreilly commented Jul 6, 2019

johnnyreilly commented Jul 7, 2019

phryneas commented Jul 7, 2019

johnnyreilly commented Jul 7, 2019

phryneas commented Jul 7, 2019 •

edited

Loading

johnnyreilly commented Jul 7, 2019

piotr-oles left a comment

johnnyreilly commented Jul 8, 2019

phryneas left a comment

johnnyreilly commented Jul 9, 2019 •

edited

Loading

johnnyreilly commented Jul 10, 2019 •

edited

Loading

johnnyreilly commented Jul 11, 2019

piotr-oles left a comment

johnnyreilly commented Jul 11, 2019

piotr-oles commented Jul 11, 2019

johnnyreilly commented Jul 11, 2019

johnnyreilly commented Jul 11, 2019

piotr-oles left a comment

johnnyreilly commented Jul 12, 2019

piotr-oles commented Jul 12, 2019

johnnyreilly commented Jul 12, 2019

johnnyreilly commented Jul 13, 2019

add eslint typescript support #305

add eslint typescript support #305

Conversation

johnnyreilly commented Jun 30, 2019 • edited Loading

phryneas commented Jul 2, 2019

johnnyreilly commented Jul 2, 2019 • edited Loading

johnnyreilly commented Jul 6, 2019

johnnyreilly commented Jul 7, 2019

phryneas commented Jul 7, 2019

johnnyreilly commented Jul 7, 2019

phryneas commented Jul 7, 2019 • edited Loading

johnnyreilly commented Jul 7, 2019

piotr-oles left a comment

Choose a reason for hiding this comment

johnnyreilly commented Jul 8, 2019

phryneas left a comment

Choose a reason for hiding this comment

johnnyreilly commented Jul 9, 2019 • edited Loading

johnnyreilly commented Jul 10, 2019 • edited Loading

johnnyreilly commented Jul 11, 2019

piotr-oles left a comment

Choose a reason for hiding this comment

johnnyreilly commented Jul 11, 2019

piotr-oles commented Jul 11, 2019

johnnyreilly commented Jul 11, 2019

johnnyreilly commented Jul 11, 2019

piotr-oles left a comment

Choose a reason for hiding this comment

johnnyreilly commented Jul 12, 2019

piotr-oles commented Jul 12, 2019

johnnyreilly commented Jul 12, 2019

johnnyreilly commented Jul 13, 2019

johnnyreilly commented Jun 30, 2019 •

edited

Loading

johnnyreilly commented Jul 2, 2019 •

edited

Loading

phryneas commented Jul 7, 2019 •

edited

Loading

johnnyreilly commented Jul 9, 2019 •

edited

Loading

johnnyreilly commented Jul 10, 2019 •

edited

Loading