Skip to content
This repository has been archived by the owner on Nov 16, 2023. It is now read-only.

Doodles: attempt to add ESLint, round 3 #341

Closed

Conversation

JoshuaKGoldberg
Copy link

Kind of a conglomeration of #305 and #325. In progress:

A few notes:

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. 馃

@JoshuaKGoldberg JoshuaKGoldberg marked this pull request as draft August 22, 2021 20:47
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.");
Copy link
Author

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 馃槥

Copy link
Contributor

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

Copy link
Author

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?

Copy link
Contributor

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

@43081j
Copy link
Contributor

43081j commented Aug 24, 2021

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`
Copy link
Member

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.
Copy link
Author

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...

@JoshuaKGoldberg
Copy link
Author

Closing as this has aged away. If you the reader would like this code, feel free to take it and send your own PR. 鉂わ笍

@JoshuaKGoldberg JoshuaKGoldberg deleted the switch-to-eslint-3 branch April 3, 2022 23:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants