-
Notifications
You must be signed in to change notification settings - Fork 86
Doodles: attempt to add ESLint, round 3 #341
Doodles: attempt to add ESLint, round 3 #341
Conversation
console.error("Args:"); | ||
console.error(" --version Print version and exit."); | ||
console.error(" --installAll Cleans and installs all TypeScript versions."); | ||
console.error(" --expectOnly Run only the ExpectType lint rule."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @43081j - out of curiosity, did you end up looking at this? I'm a little irked at how complex this is in the scenario I outlined in the OP 😞
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we'd be programmatically calling eslint, right? wouldn't we be able to specify the rule set to run in that case, to just this rule? on the command line, you'd have to do it by a custom eslint or --no-eslintrc
i think. but in code i'd assume you can pass the rule in by itself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah my sadness there is that if someone has a deeply nested set of directories, each with their own .eslintrc.json
, we'd have to compute from each of those config files what the actual set of rules to run is. Each ESLint config could specify something different for an expect-type* rule.
Maybe I'm overthinking this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so, what I'm saying is we will be programatically calling it anyway. So just like in unit tests we could enable only one rule (expect type rule). Ignoring any user defined rule sets. I think it'd be part of dtslint rather than the plugin holding the rules
i have an unpushed branch somewhere with a bunch of the rules converted, ill push that tonight in case its useful 👍 |
@@ -131,14 +121,6 @@ Use your locally installed version of TypeScript. | |||
```sh | |||
dtslint --localTs node_modules/typescript/lib types | |||
``` | |||
- `--expectOnly` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expectOnly is used by dtslint-runner to speed up linting in incremental mode for dependents, so that only packages a PR modifies actually run all the lint rules.
Otherwise CI runs for node, react, et al take too long.
@@ -1,5 +1,5 @@ | |||
`dtslint` tests a TypeScript declaration file for style and correctness. | |||
It will install `typescript` and `tslint` for you, so this is the only tool you need to test a type definition. | |||
It will install `typescript` and `eslint` for you, so this is the only tool you need to test a type definition. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Todo: for now, it will still install tslint
as well...
Closing as this has aged away. If you the reader would like this code, feel free to take it and send your own PR. ❤️ |
Kind of a conglomeration of #305 and #325. In progress:
@typescript-eslint/eslint-plugin-tslint
A few notes:
--expect-only
would require running ESLint separately on every file or folder of files. Is that something we really want to keep?--rulesDir
ecosystem works a bit differently from TSLint's--rules-dir
and likely will make it harder to position adtslint
as the "only" dependency to be installed.eslint-plugin-local-rules
exists as a result.I need to think a bit more; posting this up for now as a scratchpad... will move all these todo-style notes into a
<details>
tag and make a real PR description once that's done. 🤔