add support for eslint

[lddubeau]

Currently, tide provides support for linting with tslint but not for linting with eslint. It turns out that linting TypeScript code with eslint is already a thing. (I did not realize that. I use eslint but only for JS code, and I exclusively use tslint for TS code.)

I think adding support for eslint now would be an enhancement, but it appears that tslint will eventually be deprecated. (See microsoft/TypeScript#29288.) So eventually the lack of support for eslint will be a bug.

I see 3 areas of concern:

1. When eslint is to be used for linting, sequence the eslint-based checker to run after typescript-tide and tsx-tide like we do with typescript-tslint.

2. Provide an eslint equivalent to tide-add-tslint-disable-next-line. (I actually already have code for this.)

3. Update the documentation to explain how to use eslint with tide. (The equivalent of what we already have for tslint.)

[ananthakumaran]

I am ok with 2 and 3.

about 1 could you explain how would you figure out if the user is planning to use eslint? via custom variable config?

Personally, I don't like the fact that tide has to be responsible for setting the correct next checker. We had the discussion about the same issue before and finally agreed to set next checker.

Yeah, it seems tide having better performance, but I've been having problem that it keeps creating a flycheck_myfilename.tsx file, and my react server keeps breaking :( , I am not sure if it's flycheck's problem or tide's. That's why I switched to lsp, it's been working better than tide tbh, just a bit slow :(

but apparently it's confusing others as well. But, I don't have any good alternative solution either for the issue.

[josteink]

That's why I switched to lsp, it's been working better than tide tbh

Disregarding everything else about this issue, I can really sympathize with this, and it's been a thought I've been having for a good while.

Once Emacs and LSP works properly, why should we bother maintaining a separate LSP client, just specifically for TypeScript?

When I checked the state of affairs with Emacs and LSP last year, it was quite a joke. Recently though I've taken a look at it again (from a Rust-perspective) and it has improved immensely, to the point of me suspecting that whatever I was missing was actually due to incomplete support in the Rust Language Server, not Emacs.

And once things get really good and just work out of the box, I would really expect most Emacs-users to prefer a "standard" LSP experience over some non-standard thingie like tide, also for TypeScript.

Basically at some point in the not too distant future I expect (hope?) tide to actually become obsolete, because at that point it would be a good thing... Right?

Is that just me, or is anyone else also considering this future possibility?

Which leaves me with some mixed feelings about investing too much extra effort into improving tide at this point. I'd rather help get "official" LSP-mode working flawlessly for TypeScript out of the box, than invest more in something soon to be obsolete.

And when the time comes that LSP-mode is the obvious choice, I think it would make sense to port whatever we have which is tide-specific (and not bound to LSP) into basic TypeScript mode.

Is this something anyone else thinks we should look into? Or is it just me?

(Also: Should this be discussed in a separate issue on its own?)

[ananthakumaran]

I agree, in the long run, LSP is the way to go. Then the question is mostly when?. I have been using LSP for other languages and the emacs client has improved a lot in recent days, but the server implementations are not up to par with tsserver. I have been using elixir lsp where even jump to definition breaks randomly. If TypeScript officially switches to lsp, then we can deprecate this package. Visual Studio Code probably has the exact same problem, they use tsserver but also support lsp for other languages I believe.

[lddubeau]

@ananthakumaran

about 1 could you explain how would you figure out if the user is planning to use eslint? via custom variable config?

What I have in the first post is a wish list. I've not put any thought into the details of how it would be implemented. I start from the position is that eslint support should be at parity with tslint support. It should work right out of the box. If there are insurmountable technical obstacles, then we have to depart from that.

I filed the issue because that's likely to become a pressing issue if no work is done towards supporting eslint. Now it is on the radar. It is not a pressing issue for me, but maybe there's someone among tide's users who is already using eslint for linting TypeScript and for whom this issue is more pressing.

And I agree with what was expressed regarding LSP vs Tide.

[shroomist]

I'd like to see elsint with tide on my TS.

[mk0x9]

Silly question, but what's tide is doing with tslint? Why there is need from tide's side to eslint while we have flycheck? Except for tide-add-tslint-disable-next-line.

[lddubeau]

@mk0x9 We have things like this:

(flycheck-add-next-checker 'typescript-tide '(warning . typescript-tslint) 'append)

The typescript-tslint checker is part of flycheck, but tide is sequencing the checkers so that tslint runs once tsserver stops reporting compilation errors. This is point one in the list of three points I put in the initial post. I assume that just like we do with tslint, eslint will run after tsserver no longer reports compilation errors. And ideally, tide would set things up so that whatever linters are configured for a project run without requiring the user to specifically tell tide what to run.

[talwrii]

Silly question, but what's tide is doing with tslint?

Projects will often include a lint step as part of their continuous integration. if this is the case, it's convenient to display the errors that will cause your build to fail in emacs.

tslint is to some degree deprecated

https://medium.com/palantir/tslint-in-2019-1a144c2317a9
The motivation being that over time ecma is swallowing features from tide, to avoid implementing things twice it makes sense to use a tool that supports both ecma and typescript. This way, when ecma adopts a feature it is easy to support it.

[emmanueltouzery]

so how would I configure tide to use eslint, not tslint to validate my ts/tsx files? If I run this command in an already opened ts/tsx buffer, it works:

(flycheck-add-next-checker 'typescript-tide 'javascript-eslint 'append)

however, I don't manage to configure this through my emacs config. I tried adding that line as-is, but it didn't work, nor did this, trying to tie it to the major mode through a hook:

(add-hook 'typescript-mode
          (lambda ()
            (flycheck-add-next-checker 'typescript-tide 'javascript-eslint 'append)))
(add-hook 'typescript-mode
          (lambda ()
            (flycheck-add-next-checker 'tsx-tide 'javascript-eslint 'append)))

so how could I configure this? Thank you!

[lddubeau]

@emmanueltouzery The way I have it in my .emacs file I run the flycheck-add-next-checker commands after flycheck, typescript-mode and tide are loaded. But I don't put those commands in a hook.

By the way, I've been working on adding eslint support to tide. I've got point 2 of my list of points in the original post pretty much done. I've yet not done much regarding point 1, other than getting a configuration that WorksForMe(tm) but is probably not general enough.

[emmanueltouzery]

@lddubeau thanks for the feedback! Looking forward to builtin support in tide.

For now I made it work in my case:

;; https://github.com/hlissner/doom-emacs/issues/1530#issuecomment-507653761
(add-hook 'typescript-mode-local-vars-hook
          (lambda ()
            (flycheck-add-next-checker 'typescript-tide 'javascript-eslint 'append)))
(add-hook 'typescript-mode-local-vars-hook
          (lambda ()
            (flycheck-add-next-checker 'tsx-tide 'javascript-eslint 'append)))

i use doom-emacs so I don't have complete control when things get loaded. -local-vars-hook is a doom-emacs thing.

[lddubeau]

I've been using my code for a while with a mixture of projects, with various scenarios. I've run into some complications, and I'm having some doubts as to whether and to what extent this project should tackle what I had identified as points 1 and 3 in my original post:

1. When eslint is to be used for linting, sequence the eslint-based checker to run after typescript-tide and tsx-tide like we do with typescript-tslint.

3. Update the documentation to explain how to use eslint with tide. (The equivalent of what we already have for tslint.)

There are two complications:

1. The number of linters a user may run into from project to project is unlimited. There's eslint, tslint, vue-cli-service lint, ng lint, and the other frameworks probably have their own lint tools too.

   When a framework linter is used, it usually boils down to eslint or tslint being invoked so as far as my original point 2 is concerned, the variety of linting tools can usually be boiled down to a few simple cases. In some cases there may be minimal configuration required to identify a linter properly, but nothing terribly tricky.

   But the variety of linters is a problem when combined with the next complication.

2. tide relies on flycheck to report errors to the user. Getting a flycheck configuration to "just work" from project to project is a hurdle. Since I've started using eslint for linting TS projects, I've run into a few situations where the feedback I got from my tide buffer did not agree with my expectations. So I had to adjust my Emacs configuration in this way and that way, etc. And the way flycheck works, the changes I make for project A are likely to break something for project B. At this moment I'm not seeing how I could set up my Emacs configuration to JustWork(tm) in all the projects I have a hand in. Up until now, my Emacs configuration just worked in all the projects I worked on. I never had to go tweak flycheck settings for this or that project.

   There are solutions but they are either brittle, hacks, suck in some way, or all of the above. The risk I foresee those of us maintaining tide is to be faced with a flood of questions about how to get flycheck to behave in this case, and that case, and that case, and that case.

[josteink]

The number of linters a user may run into from project to project is unlimited.

While maybe not unlimited, it's clearly non-zero and more than one. And that's when we're not even accounting for which versions are in use. Even linters have breaking changes across major versions every now and then.

tide relies on flycheck to report errors to the user.

Which in some cases makes perfect sense, at least for errors reported by tsserver.

But... for linters?

Getting a flycheck configuration to "just work" from project to project is a hurdle.

I don't have the experience to either confirm or contradict this... But wouldn't it make more sense to move support for the linting bit into flycheck itself, or dedicated flycheck packages?

Because asking a question which can easily be googled is kinda of lazy, I decided to investigate that bit slightly, and I found this:

• flycheck-typescript-tslint... Which reportedly is obsolete and already part of flycheck... So then we don't have to support it then?
• It also seems like flycheck handles eslint by itself... Because there's issues registered for eslint-related bugs.

So ... In what ways are tide responsible for linting behaviours? Am I missing something?

Shouldn't linting-related errors go to flycheck for now?

[lddubeau]

The number of linters a user may run into from project to project is unlimited.

While maybe not unlimited, it's clearly non-zero and more than one.

I meant that you cannot put an arbitrary limit on the number of linters that may be used for linting TS code. If you state "there are X linters for TypeScript", I can assure you it is only a matter of time before someone comes up with a new framework that spins its own linting tool wrapping around eslint.

And that's when we're not even accounting for which versions are in use. Even linters have breaking changes across major versions every now and then.

In my experience the breaking changes across major versions have not been particularly challenging to handle. And the fixes where squarely on the flycheck side of the tide/flycheck dividing line of responsibilities.

tide relies on flycheck to report errors to the user.

Which in some cases makes perfect sense, at least for errors reported by tsserver.

But... for linters?

Getting a flycheck configuration to "just work" from project to project is a hurdle.

I don't have the experience to either confirm or contradict this... But wouldn't it make more sense to move support for the linting bit into flycheck itself, or dedicated flycheck packages?

Unless there's a major change in development philosophy from the dev(s) of flycheck, I don't see flycheck itself being the home for this. Getting the typescript-tslint checker to be fixed has been pointless. The issue reports are there, but nothing is happening. I've used typescript-tslint for a very short span of time, until I ran into the inevitable bugs and defined my own checker.

I've checked whether there's any desire to include in flycheck something that would do what tide-add-tslint-disable-next-line does, and saw discussions that went nowhere.

It is true that a flycheck-... package could be spun off without needing anyone's approval.

Because asking a question which can easily be googled is kinda of lazy, I decided to investigate that bit slightly, and I found this:

• flycheck-typescript-tslint... Which reportedly is obsolete and already part of flycheck... So then we don't have to support it then?

In practice the actual support for tslint in flycheck is laughable. It works for toy projects, but as soon as it meets the real world, problems start surfacing:

• It entirely ignores the --project and --type-check flags. --type-check has been deprecated, and is presumably no longer necessary, but for a period of time it was mandatory to get rules that do type checking to work. --project though is still necessary for tslint to take into account the tsconfig.json and take type into accounts. This problem has been reported to the flycheck issue tracker: no reaction.

• It uses source-inplace which causes problems. Ironically, the fact that flycheck does not use the flags I mentioned above hides the issues with source-inplace to some extent. This has been reported: no reaction.

• I treats all tslint reports as warnings when some are errors and some are warnings. I don't know whether this has been reported.

• It entirely ignores the Node ecosystem convention that when building a project the tools used are those installed locally with the project.

• It also seems like flycheck handles eslint by itself... Because there's issues registered for eslint-related bugs.

I suspect eslint is better supported than tslint but I can foresee issues similar to those that happen with tslint recurring here too. In particular, I know for a fact that right now flycheck ignores the Node convention I mention above. I got it to work for me by writing an eslint script that finds the correct eslint executable to run.

So ... In what ways are tide responsible for linting behaviours? Am I missing something?

Shouldn't linting-related errors go to flycheck for now?

They are already going to flycheck and no one is proposing otherwise.

However, tide has not up to now taken a hands off approach when it comes to linters. Tide has already taken upon itself to provide support for a certain development setup, one which is no longer being maintained. Namely:

1. tide automatically sets (flycheck-add-next-checker 'typescript-tide '(warning . typescript-tslint) 'append). This can be beneficial. Or this can mess up someone's flycheck setup. Earlier, when tslint was the de facto standard linter for TS, it was much more likely to be beneficial than harmful. But now it is really a toss of the coin. It depends on the order in which things are loaded and configured by the developer who uses Emacs, and on the specifics of the project he's working on. Because that call to flycheck-add-next-checker could make it so that flycheck is going to run tslint instead of whatever other linter the developer might need. (Flycheck iterates over the next checker list and uses the first checker it can use. The rest is ignored.)

   But wait.... if a project uses eslint rather than tslint for linting TS code, then tslint is not installed and there should not be a conflict, right? Wrong. As we speak, there's a substantial set of tslint rules that are not supported natively in eslint. They are supported by having eslint invoke tslint behind the scenes. So even in a project that does not use tslint directly, tslint may still be installed, and configured, etc.

2. tide has tide-add-tslint-disable-next-line which supports tslint but not eslint.

Letting users figure out how they need to configure flycheck is fine but, then in my opinion, if that's the approach adopted, tide should do nothing more than define the checkers it needs to report tsserver errors. In particular, it should not tell flycheck to run tslint after tide's checker has run. It is up to the user to tell flycheck to do that. Just like they would for eslint, or vue-whatever, or some other tool that wraps around eslint, or tslint.

I think tide-add-tslint-disable-next-line can be still supported as proposed in #359 though an argument could be made that this does not belong in tide either.

[josteink]

Thanks for the thorough and detailed reply @lddubeau. Greatly appreciated.

The current situation (at least from a code-sanity perspective) honestly sounds about as sad and depressing as it could possibly be.

So if I understand things correctly:

• flycheck claims to handle tslint, but does it so poorly so that it's not even usable. Working it out upstream does not seem like a productive endeavour.
• flycheck may handle eslint better, but there are room for certain improvements (like respecting node conventions). Might be fixable upstream, if we contribute?
• tide has some support for tslint, but it's inconsistent, and not guaranteed to provide a better experience for the end-user.
• even if we somehow sort all that stuff out inside tide, more linters and configurations may come and complicate matters later.

If that understanding is anywhere near accurate I propose:

• try to work out eslint problems upstream, PRs over issue-reports.
• try to spin out tslint support into a dedicated (new) "flycheck-tslint"
• other linters which may come later can either be supported by flycheck-tslint. Maybe it could be flycheck-ts-lint and support a wider array of typescript-related linters?
• remove existing linting code from tide, but document the best way to get working with popular linters and configs (what packages are required, etc)

While I think sounds like more work for end-user, it sounds like a much more composable solution, and not to mention a solution where it will be easier for end-users to put together a working solution for their own projects, and have it keep working.

Agreed? Or any objections?

[lddubeau]

@josteink I agree. I would prioritize the dedicated flycheck-ts-lint package over trying to push changes into upstream. Unless you think you can quickly become a collaborator to flycheck and champion TypeScript support from inside the flycheck team, I don't expect that trying to push changes into the flycheck project itself is going to be very satisfactory.

Also, it is still unclear to me what form the support for TS linters should take in the long run. In other words, whatever I'd send to upstream would feel more experimental than I'd like. Which could prove another obstacle in the short term to pushing changes upstream if the people evaluating the changes also find them too experimental.

Of course, nothing prevents having both flycheck-ts-lint and pushing some selected changes to upstream.

[M3kH]

Seems that the road is establish to convey on eslint.

As an update, is worth talking about a plan to facilitate a migration?

[rymndhng]

I noticed that tslint has been removed in #413 🎉.

Going back to the original issue, would now be a good time to address point no.2? This function is unbelievably helpful.

2. Provide an eslint equivalent to tide-add-tslint-disable-next-line. (I actually already have code for this.)

[josteink]

2. Provide an eslint equivalent to tide-add-tslint-disable-next-line. (I actually already have code for this.)

I guess that leaves you @lddubeau to be a man of your word? 😄

[lddubeau]

Here's the situation. I'm dealing with a serious health condition that has taken me out of rotation since June 2020, and will probably continue until November 2021. I can read emails but I have not gone back to working on github yet. It's hard to tell when I'm going to be back to my old self since I still have one major operation to undergo in April. They say 6 months recovery time but I suppose I'll be able to get back to giihub earlier than that. The 6 months figure includes physical activity for instance. But I cannot tell when that will happen. I wouldn't be offended if you all decided to move forward without me.

…

On Mon, Mar 1, 2021 at 2:45 AM Jostein Kjønigsen ***@***.***> wrote: 1. Provide an eslint equivalent to tide-add-tslint-disable-next-line. (I actually already have code for this.) I guess that leaves you @lddubeau <https://github.com/lddubeau> to be a man of your word? 😄 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#308 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAO7LZPYUA4GLDFAO3ZJXMLTBNAZBANCNFSM4HAGPWFA> .

[josteink]

Take care of yourself and take your time. Certainly do NOT worry about silly GitHub issues! 😆

I hope all goes well and wish you the speediest of recoveries! ❤️

[rymndhng]

Take care @lddubeau!

[lddubeau]

Thanks guys!

[josteink]

I guess that means that if anyone else wants to take a stab at this, any PR is greatly welcomed :)

[brprice]

I noticed that tslint has been removed in #413

However, tslint is still mentioned a couple of times in the readme: M-x tide-add-tslint-disable-next-line and (flycheck-add-mode 'typescript-tslint 'web-mode). I have set my environment up with adding javascript-eslint checker, and I see that tide-add-eslint-disable-next-line is already a thing. At first glance these both seem to work fine (I get (typescript-specific) eslint warnings in my buffer, and can add the relevant comment to disable them).

Is updating that documentation (assuming it actually works as well as it initially appears to) all that is left of this issue, or have I missed something?