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

Move from internal expect rule to eslint-plugin-expect-type #858

Open
3 tasks
JoshuaKGoldberg opened this issue Dec 11, 2023 · 3 comments
Open
3 tasks

Move from internal expect rule to eslint-plugin-expect-type #858

JoshuaKGoldberg opened this issue Dec 11, 2023 · 3 comments

Comments

@JoshuaKGoldberg
Copy link
Contributor

JoshuaKGoldberg commented Dec 11, 2023

#843 migrated the old TSLint Expect rule to an ESLint rule (🥳!). Great!

There exists a community plugin for roughly the same area of functionality already: https://github.com/JoshuaKGoldberg/eslint-plugin-expect-type 1. It would be great if we could use the standard plugin instead. That way bugfixes + features from DT usage can help other community members, and vice versa.

Proposal:

  1. For each bug or feature that is better in this repo, we can file a corresponding issue on eslint-plugin-expect-type
  2. Once those are all fixed up, move to using the external plugin

Per later comments, tracking issues in eslint-plugin-expect-type for missing features:

Footnotes

  1. I do feel a bit weird promoting a package with my name on it. If it helps, I didn't create it - just took over maintenance when the previous maintainer didn't have time. If there's a better alternative to eslint-plugin-expect-type that'd be great too!

@jakebailey
Copy link
Member

jakebailey commented Dec 11, 2023

I'm not sure how we can do that. Our plugin has to loop and run on every version of typescript. That seems like a lot of extra work for an external plugin to take on (and for us to no longer be able to quickly fix).

@JoshuaKGoldberg
Copy link
Contributor Author

At the very least, I wonder if eslint-plugin-expect-type could export a Node API (or one be extracted out, or some similar strategy) to deduplicate the code. Then it could take in getProgram as an option... 🤔

Alternately, I wonder if it would make sense to consider versionsToTest "just another" option for the rule? As in, copy that logic roughly as-is to eslint-plugin-expect-type? DefinitelyTyped is likely not the only real-world use case for wanting to run across multiple TS versions.

@jakebailey
Copy link
Member

The only interesting things missing from your plugin are || (for multiple possible answers), and the normalization of union ordering / readonly array syntax (https://github.com/microsoft/DefinitelyTyped-tools/blob/master/packages/eslint-plugin/src/rules/expect.ts#L492), both of which are related to running on multiple versions of TypeScript.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants