Skip to content

Converting rules to be in TypeScript #47

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

Closed
aboyton opened this issue Aug 20, 2017 · 11 comments · Fixed by #120
Closed

Converting rules to be in TypeScript #47

aboyton opened this issue Aug 20, 2017 · 11 comments · Fixed by #120
Assignees
Labels
enhancement New feature or request package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@aboyton
Copy link
Contributor

aboyton commented Aug 20, 2017

I was wondering people's thoughts of switching this repository over to using TypeScript. Personally I find it much easier to code using TypeScript, especially on an unfamiliar codebase. One of the nice things about developing rules on TSLint is that you can write rules in either TypeScript or JavaScript.

I was thus curious had we considered allowing people to write rules with TypeScript?

@j-f1
Copy link
Contributor

j-f1 commented Aug 20, 2017

Things I encountered when trying to write custom ESLint rules in TypeScript for desktop/desktop:

  • There’s no complete typings for the AST. (there are typings for ESTree in @types/estree, but they don’t cover ESLint/espree-specific changes nor do they cover the TypeScript nodes)
  • There’s no @types for ESLint. This makes it hard to use context.report() etc.
  • I’m still looking for a way to require that the exports of a module conform to a specific interface. A way to do this would be great!
  • It can be difficult to narrow down the types for nodes. It might be advantageous to create a module similar to babel-types that has methods like isIdentifier(Node: node): node is Identifier to narrow down types.

@corbinu
Copy link
Contributor

corbinu commented Aug 24, 2017

I think I am negative one on this one as much as I wish the eslint project was in TypeScript it is not and I think it is better to keep this project in line with the style of eslint as a whole for consistency :(

@aboyton
Copy link
Contributor Author

aboyton commented Aug 28, 2017

There's definite advantages either way. There's something nice about using the same language as the rest of ESLint, and there's something nice about writing rules in the same language that they are running on. It definitely very common for tooling to be written in the language the tooling is for.

Obviously this is not my project, hence why I was asking the question and wondering what others think.

Th @types issues presumably could be/would be worked on if more people started writing TypeScript code that built on ESLint. I'm with @corbinu though, let's convert ESLint over to be in TypeScript 😛

@bradzacher
Copy link
Member

I've done typescript + eslint config before (https://github.com/assignar/eslint-config-assignar), but I haven't done a plugin with it yet.

There's obvious benefits about switching to typescript which could help catch some bugs that were missed.
Might be a bit of work to define our own types if any are missing.

Would be worth investigating

@johnwiseheart
Copy link
Contributor

I think now that typescript in ESLint is becoming a lot bigger of a thing, this is worth re-visiting. @bradzacher would you support PR(s) to start converting rules to typescript?

@bradzacher
Copy link
Member

Yes and no.

On this repo specifically, no. We're getting ready to migrate this repo to the monorepo.

I definitely want to migrate this project into typescript if only because we've had a few bugs caused by types.

So once we have migrated, we can tackle this.

@johnwiseheart
Copy link
Contributor

Sounds good, going to create an issue on the monorepo regarding documenting writing rules for ts in ts, and then once this repo migrates over there we can follow it up.

@JamesHenry JamesHenry transferred this issue from bradzacher/eslint-plugin-typescript Jan 18, 2019
@bradzacher bradzacher added enhancement New feature or request package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin labels Jan 18, 2019
@bradzacher bradzacher self-assigned this Jan 21, 2019
@43081j
Copy link
Contributor

43081j commented Jan 21, 2019

Hello 👋

Just as an FYI, we currently write our rules in typescript over at 43081j/eslint-plugin-lit so I had a poke around this repo too to see how difficult it is to do the same.

It is all fairly straight forward using @types/estree and @types/eslint. However, you will struggle when dealing with the custom AST nodes we make use of here (e.g. TSDeclareFunction). If you exported your AST node types in a typescript definition file somewhere, you could possibly do some interface merging to sort this.

Also turning on all strict options and inheriting the base tsconfig.json you have seems to work nicely once util is cleaned up to be stronger typed.

Couple of tips to whoever tackles this (happy to give it a go if that's nobody):

  • Rules should be a Rule.RuleModule
  • create(context) should return a Rule.RuleListener
  • listeners take an ESTree.Node
  • requireindex has no types so you'll need a custom d.ts file for it
  • configs/* files should all export an object and be typescript rather than json i guess

@bradzacher
Copy link
Member

I started working on this today! branch:migrate-plugin-to-ts
The problem is that we don't have type definitions for the extensions and new nodes added by typescript-estree yet.

Don't hugely want to manually write them all out; there's a quite few cases to make sure we've covered correctly, plus we'd have to keep them in sync.

@armano2 has been working on ways to auto generate it.

We just need to decide which is the best way forward for that.

@43081j
Copy link
Contributor

43081j commented Jan 21, 2019

Ah great I'll take a look through the branch.

We should probably generate the types at build time like Babel does. They simply iterate through an array of node types and output a typescript file.

I'd also recommend the strict settings in typescript so we maintain nice strong types (especially no implicit any).

Also looking at your branch it would be a nicer structure I think if you remove src/lib and move its contents up a level. It's a little redundant and makes it match up with convention then (src as sources, lib as output).

@43081j
Copy link
Contributor

43081j commented Jan 21, 2019

A pr would be nice btw, as I have a few other suggestions (I know it's early stages 🙊)

@typescript-eslint typescript-eslint locked as resolved and limited conversation to collaborators Feb 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants