Skip to content
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

Support .eslintrc.ts #12078

Closed
G-Rath opened this issue Aug 8, 2019 · 25 comments
Closed

Support .eslintrc.ts #12078

G-Rath opened this issue Aug 8, 2019 · 25 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion auto closed The bot closed this issue core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion

Comments

@G-Rath
Copy link
Contributor

G-Rath commented Aug 8, 2019

The version of ESLint you are using.
6

The problem you want to solve.
Allowing typescript users to write strongly typed .eslintrc configuration files that "just work" in the same manner as .eslintrc.js.

Your take on the correct solution to problem.
use ts-node to bootstrap .ts files if it's available.

This would be following in the footsteps of other libraries, including:

While I've used ts-node, I've never implemented this sort of feature before.
I know that webpack-nano uses rechior to achieve this feature.
That in turn uses interpret, which is also what webpack uses.

I would additionally propose this happens by default, w/o requiring a config value - eslint would search for a .js file first, followed by a .ts file, since in TS land this usually represents a compiled TS file.

Are you willing to submit a pull request to implement this change?

Gladly, but would most likely require some guidance from others having never worked w/ eslint core directly (including general github & ci processes).


Overall, I think that this feature makes sense to support given the work being done to merge tslint into eslint via @typescript-eslint.

Ultimately, I'd love to see a day where I can write a .eslintrc.ts and have everything work as they would if I'd had a .js as the extension.

I think it's reasonable to require the installation of additional dependencies in the application being linted for this feature; as such if there's a way to implement this feature as a separate package over at @typescript-eslint then I'm game for that 😄

@G-Rath G-Rath added core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint triage An ESLint team member will look at this issue soon labels Aug 8, 2019
@G-Rath
Copy link
Contributor Author

G-Rath commented Aug 8, 2019

@SimenB @shellscape @bradzacher hope you guys don't mind me pinging you, but I'd love to know your thoughts on this, given your prior OSS art :)

@ljharb
Copy link
Sponsor Contributor

ljharb commented Aug 8, 2019

why would eslint support config in a language that’s handled by a third party parser? there’s no .eslintrc.flow either.

@shellscape
Copy link

shellscape commented Aug 8, 2019

@G-Rath I don't think you're going to find a sympathetic ear on the ESLint maintainers list for this one (previous somewhat snippy comment as evidence). I also don't really see value in TypeScript being used, unless you're doing some really fancy juggling in your ESLint configs, which I've never personally seen before. That being said, it should be relatively easy to create a wrapper CLI that uses rechoir to load the config and forward that onto ESLint proper. I'd suggest that as an alternate route.

@G-Rath
Copy link
Contributor Author

G-Rath commented Aug 8, 2019

@ljharb

there’s no .eslintrc.flow either.

No, but there's also no @flow-lint.

why would eslint support config in a language that’s handled by a third party parser?

Why would it support YAML when it requires another package?

I don't want eslint to support full blown typescript parsing - but given all that's needed is chucking it into ts-node/register, I don't think it's any greater than asking for yaml support.


@shellscape

That's how I thought it'd be, but since I couldn't find an issue anywhere already discussion this feature, I figured might as well open an issue and get some input.

I also don't really see value in TypeScript being used, unless you're doing some really fancy juggling in your ESLint configs.

That's fair - The primary value is consistent feature support - if I can have everything in TypeScript, then I've able to use the same features in all my code.

In addition, I can't import other TS files in; this came up during my conversion of eslint-plugin-jest to TypeScript, which resulted in me having to break an object out into a json file so that it could be shared by both the plugin & it's eslint configuration.


Overall, this issue is meant to serve as a catalyst - I don't want eslint to have this feature if it requires it to pull down TypeScript as a dependency, or other such big changes, but given what I've seen of ts-node/register, it seems like a relatively small request as it's taking advantage of already installed packages.

End of the day, I had to open an issue somewhere, and given I'm running the eslint command, figured this was a good place to start :)

@kaicataldo
Copy link
Member

@G-Rath @shellscape Hi! Please note that while @ljharb's opinion is valued here, they're not currently maintaining ESLint. I don't know if we would end up accepting this or not, but characterizing the maintainers in that light before any of us have had a chance to respond doesn't feel very fair to us.

Now, to the issue at hand! Maybe I'm misunderstanding something, but would you mind explaining what problem you'd like to solve by writing the configuration as a .ts file? Additionally, if ESLint isn't written in TypeScript, will you still get type hinting for the config file?

@kaicataldo kaicataldo added evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Aug 8, 2019
@kaicataldo
Copy link
Member

Looks like the above comment was edited after I posed my question. Thanks for explaining what you think the benefits are. That makes it a lot easier for us to evaluate the proposal.

While we're happy to discuss here, please also note that changes to core require an RFC before we implement them.

@g-plane
Copy link
Member

g-plane commented Aug 8, 2019

@G-Rath I'm curious about why you want to support .eslintrc.ts? Just a question, not means I oppose this proposal.

@G-Rath
Copy link
Contributor Author

G-Rath commented Aug 8, 2019

@kaicataldo Hi! Don't worry, I don't hold anything again @ljharb - and he had a valid point 🙂

To clarify, I was & still am expecting some pushback on this, as a library as big as eslint has to be very careful about what features it added. I totally understand you've got to consider all aspects.

Looks like the above comment was edited after I posed my question

Hahah yeah sorry about that. I'm done editing that comment now :)

Additionally, if ESLint isn't written in TypeScript, will you still get type hinting for the config file?

Absolutely! Typescript can consume definition files for typings, which don't have to be in the package itself.

These typings typically live at DefinitelyTyped, but given the existence of @typescript-eslint, I'd imagine they would could live there nicely as well.

I also think that @typescript-eslint might have at least some definitions for the config already done.

The types themselves I don't think should be required to be "100% accurate", in terms of having all supported rules and their configs defined.

While that would be nice, I think it would be far too much work, at least for the initial typings.
That is the other great thing: You can always refine typings over time :)

Just for reference, jest is completely TS compatible library that has externally defined types, and doesn't require any additional tricks to work (i.e TS understands its global, and so doesn't require you to import jest to consider it defined).


@kaicataldo & @g-plane

what problem you'd like to solve by writing the configuration as a .ts file

I'm curious about why you want to support .eslintrc.ts? Just a question, not means I oppose this proposal.

Those two points I mentioned in my above comments are the primary ones off the top of my head - overall I'm a big fan of having a consistent codebase where possible, as it tends to make general development flow a lot nicer since you don't have to worry as much about remembering what features you can and can't use in what files (not to mention you can copy and paste w/o having things explode on you).

I've found developers have similar experiences when working w/ Babel - the babel config can't babel itself, so you're left unable to use decorators, optional chaining, etc.

Luckly, eslint doesn't have the circular problem that prevents babel from supporting this :)

The other advantage of having everything in TS is that they can interact; You can't import a .ts file into a .js file w/o something like ts-node/register.

While we're happy to discuss here, please also note that changes to core require an RFC before we implement them.

I'm happy and willing to do as much of the work as I can - having not written an official RFC for something like eslint before, I can't promise quality, but if people are happy to review, I'm happy to code & write :)

The only issue I have w/ that is I foresee this turning into "generic support loader/registers" RFC/feature, which while I think that might be a great idea, is not one I think I have the experience & time to champion on my own.

@G-Rath
Copy link
Contributor Author

G-Rath commented Aug 8, 2019

Another (minor) gain to supporting .eslintrc.ts is the general operability w/ other major packages, like webpack.

While this comes close to being one of my original reasons, and doesn't have a direct impact on eslint, I do think there is value in the ease-of-use of being able to switch to .eslintrc.ts in the same manner you can your webpack.config.js & danger.js (and hopefully soon babel.config.js).

I also think it would be fair to say that in this case .ts is to .js in the same way .yaml is to .json. (but I'm sure people will disagree 😉)

My brain is screaming that I've missed at least one major library that supports this, but can't think of it right now :/

@bradzacher
Copy link
Contributor

I ranted a bit here, sorry.
TLDR is that there are potential UX benefits for users that use/understand typescript in the form of clearer compile-time errors.


This is an interesting thought. I've considered it before, but I don't hugely want to deviate from eslint with a cli wrapper.

We've had an jump in the number of new users - both brand new users who've never linted before, as well as those migrating from tslint.

One of the more common problems I've seen raised on @typescript-eslint is incorrect config files: misunderstanding how to configure plugins, and more importantly - incorrectly configuring rules.

Some of this could probably be handled with some clearer messaging from eslint's side - a lot of users don't understand the error when missing "severity" ("foo": {"config":""} instead of "foo": ["error", {"config":""}]) - Configuration for rule "foo" is invalid: Severity should be one of the following: 0 = off, 1 = warn, 2 = error..

But another chunk of it is that most users can find the messages from JSONSchema somewhat cryptic, esp when a rule does more complex things by combining oneOf/anyOf, etc.

Supporting .eslintrc.ts would allow us to provide tooling to type config files, and because the users are a lot used to typescript errors (compared to JSONSchema errors), it would help users more clearly understand problems at compile time.


if ESLint isn't written in TypeScript, will you still get type hinting for the config file

As @G-Rath mentioned - this doesn't matter! There are already types that have been written by the community that users can install and consume, so the additional types would just have to be added there. Eg. DefinitelyTyped has eslint types based off of the standard estree spec, and typescript-eslint has types for eslint based off of the typescript-estree spec.

Essentially a user would be able to do the following:

// consume types from DefinitelyTyped
import { RCConfig } from 'eslint';

const config: RCConfig = {
  plugins: ["jest"],
  rules: {
    indent: 'tab' // type error
  },
};

export = RCConfig;

Another great thing about this is that it's pretty easy to provide types for any and all plugins with little effort (leveraging prior art in the community).

I've done some work myself (this script automatically generating these types) to do automatic generation of types for rules by converting the JSONSchemas to typescript types (through this work I've found bugs in a number of rules - one of which was in a base eslint rule! #12051).

This would further improve the user experience, because they could get compile time errors for misconfigured rules (no matter where the rules come from).

@g-plane
Copy link
Member

g-plane commented Aug 8, 2019

TL; DR: Just a joke.


@G-Rath

I think you may want the GitHub languages status for your codebase to keep 100% TypeScript with blue bar. Like this, right?
图片

I'm also a fan of doing this.

@SimenB
Copy link
Contributor

SimenB commented Aug 8, 2019

What is it that we want changed in eslint? Supporting a --require flag where you can plug in ts-node? Finding .ts config file (and throw a nice error if the aforementioned --require flag is not passed)? Without any form of type checking, I'm not sure it's worth it to change anything within eslint itself, unless I'm missing something.
Is the thought that some sort of central drive from eslint itself would go some way in making sure the different plugins gets properly typed? Or would a -r like mocha and nyc (and probably many more) has be enough?


However, the points @bradzacher brings up are super exciting. Getting code completion (and squiggles) when configuring rules would be really nice! I'm not sure how we'd load type info for community written rules, but I also guess people better at how tsc works could tell me it's quite simple. 🙂

@G-Rath
Copy link
Contributor Author

G-Rath commented Aug 8, 2019

Supporting a --require flag where you can plug in ts-node?

The problem I have w/ making it a flag, is that it's common place to just run eslint by itself, the way way you run webpack, which means it'll instantly break and you have to know the flag syntax.

But that's also something that can be changed later, and there are methods to work around it (tho I think a lot of tool defaults to just calling eslint w/ a file path, which'd be annoying :/)


I have been thinking about it, and had an idea that I think people probably won't be too keen on,but still might work somewhat nicely: an advance babel-config-loader package.

You could work it that that eslint just looks for that package - if it's installed, it calls it like a method and calls it a day.

That way you could have purely opt in functionality w/ the minimalist of changes - if I want .eslintrc.ts, then I install ts-node & babel-config-loader, or maybe just babel-config-loader-ts.

It's just thought - one that I suspect will serve best as a strawman than a final proposal, but that'd give you a way to easily add configs of different types that the community could maintain.

Getting code completion (and squiggles) when configuring rules would be really nice!

That is exactly what I hoped would come up, and I think is a really exciting end goal.

I'm not sure how we'd load type info for community written rules

That is the interesting one. I think worst case you'd just make it a strict generic i.e

type RulesBundle = Rule[];

interface ESLintConfig<Rules extends RulesBundle> {
  rules: Rules & ESLintCoreBundledRules;
}

const config: ESLintConfig<ESLintPluginJest & ESLintPluginTS> {
  rules: [
     'foo' // error! 'foo' is not defined in ESLintPluginJest & ESLintPluginTS & ESLintCoreBundledRules
  ]
}

As overly verbose as it seems, I think that'd be an acceptable trade off. It'd actually work rather nicely, b/c it could enforce things.

I just realised maybe you could do it by just doing something like:

const config: ESLintConfig & ESLintPluginJestConfig & ESLintPluginTSConfig // and so on...

@ljharb
Copy link
Sponsor Contributor

ljharb commented Aug 8, 2019

With the checkJs flag, couldn’t typescript (possibly with some changes) just check a JS config as if it’s TS? that would bypass the problem of pegging eslint to a typescript version, and would still provide the benefits @bradzacher mentions.

@bradzacher
Copy link
Contributor

With the checkJs flag, couldn’t typescript (possibly with some changes) just check a JS config as if it’s TS?

checkJs will just do its best to infer types for .js files, but it doesn't enable you to write proper type annotations and the like. It'll use JSDoc to infer types as well.

Which means that typescript would just treat this as an object literal with no checks, because without type annotations, it doesn't "mean" anything special.

const config = { rules: { /* ... */} };
module.exports = config;

I believe that you could get around this easily(ish) by creating a "noop" function to add types. The function would have a type definition attached with the module types, so I believe that typescript would be able to use it to typecheck the provided config:

// @typescript-eslint/utils
export function createESLintRC(config: ESLint.Config): ESLint.Config {
  return config;
}


// .eslintrc.js
const { createESLintRC } = require('@typescript-eslint/utils');
module.exports = createESLintRC({ rules: { /* ... */} }); 

I'm not sure how we'd load type info for community written rules

You could leverage declaration merging to make it work ambiently by just installing the plugin types (eg @types/eslint-plugin-jest): tsplayground repl

@kaicataldo
Copy link
Member

kaicataldo commented Aug 8, 2019

Thanks for all the information - I have a much better understanding now!

Some thoughts:

  1. Being able to type check the configuration would be a very cool feature! 😄
  2. It seems unlikely to me that the team could commit to maintaining type definitions for every release (though if we wanted to pursue that, we definitely could bring it up in an RFC or bring it to the TSC). I know the idea is currently to use 3rd party type definitions, but I could see it being a (reasonable) ask if we start allowing for .eslintrc.ts configuration files in core.
  3. Out of curiosity, is this currently possible by having a .eslintrc.js file that uses the ts-node programmatic API to require and export the contents of the .eslintrc.ts file? I haven't used ts-node before, so I'm not very familiar with it.

Example:

require('ts-node').register();
module.exports = require('./eslintrc.ts');

@g-plane
Copy link
Member

g-plane commented Aug 9, 2019

I hope the TypeScript type definitions of ESLint can be maintained by our ESLint team.

@bradzacher
Copy link
Contributor

Out of curiosity, is this currently possible by having a .eslintrc.js file that uses the ts-node programmatic API to require and export the contents of the .eslintrc.ts file?

I believe that it would work as that's essentially what happens when you run node -r ts-node/register.
The obvious downside of that is that you have to have two eslintrc's.

@kaicataldo
Copy link
Member

Relevant PR and discussion: #12082

@eslint-deprecated eslint-deprecated bot added the auto closed The bot closed this issue label Sep 9, 2019
@eslint-deprecated
Copy link

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that issues failing to reach accepted status after 21 days tend to
never be accepted, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

@SimenB
Copy link
Contributor

SimenB commented Sep 9, 2019

Seeing as the above linked PR exists (even if it's about internal usage rather than public APIs), I disagree with that statement, mr bot

@G-Rath
Copy link
Contributor Author

G-Rath commented Nov 17, 2019

@kaicataldo @g-plane could this issue be reopened, given that this is being worked on & has a linked PR?

@kaicataldo
Copy link
Member

Can you point me to the PR where this is being worked on? Maybe you’re referring to something else, but the PR I linked to above relates to adding type checking to ESLint internally through comments. I thought this was valuable to this discussion since it would add TypeScript as a dev dependency to the project, but it’s not actually implementing this proposal.

If you’d like to continue championing this, would you like to create an RFC? A lot of the information can be copied over from here since this was a well-formed and discussed proposal. Thanks!

@G-Rath
Copy link
Contributor Author

G-Rath commented Nov 17, 2019

Can you point me to the PR where this is being worked on?

Apologies, I meant the general idea was being worked on ("work" in this case including discussing, thinking, etc) which the auto-closing comment from the bot I felt didn't represent.

You've answered the question I should have asked: "What is the next step to move this forward, since the team don't seem to not interested in it?"

If you’d like to continue championing this, would you like to create an RFC

I'm happy to give it a shot - I'll try to get an RFC drafted sometime this week 🙂

@kaicataldo
Copy link
Member

Thanks for clarifying. Sounds good.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Mar 8, 2020
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Mar 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion auto closed The bot closed this issue core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion
Projects
None yet
Development

No branches or pull requests

7 participants