fix(utils): removed TRuleListener generic from the createRule

[Andarist]

BREAKING CHANGE:
Removes generic parameter from public API

PR Checklist

• Addresses an existing open issue: fixes Bug: Created rules using RuleCreator are not portable #5032
• That issue was marked as accepting prs
• Steps in CONTRIBUTING.md were taken

Overview

I've dropped TRuleListener generic from the public-facing API~. So it no longer should be inferred with types coming from other packages.

This generic is still present in the RuleModule and in some other things as it seems that it's useful there for extending base rules (which kinda is like an internal thing, from what I understand).

[nx-cloud]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit f09d478. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

---

✅ Successfully ran 46 targets

• Node 18 - nx test @typescript-eslint/eslint-plugin
• Node 14 - nx test @typescript-eslint/eslint-plugin --coverage=false
• nx run-many --target=typecheck --all --parallel
• nx run-many --target=build --all --parallel --exclude website --exclude website
• nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 18 - nx test @typescript-eslint/visitor-keys
• Node 18 - nx run-many --target=build --all --parallel --exclude website --exclude website
• nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 18 - nx test @typescript-eslint/type-utils
• Node 18 - nx test @typescript-eslint/utils
• Node 18 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 18 - nx test @typescript-eslint/eslint-plugin-internal
• Node 18 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 18 - nx test @typescript-eslint/typescript-estree
• Node 18 - nx test @typescript-eslint/parser
• Node 18 - nx test @typescript-eslint/scope-manager
• Node 18 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 18 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 18 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 18 - nx test @typescript-eslint/eslint-plugin-tslint
• Node 18 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 18 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 18 - nx test @typescript-eslint/ast-spec
• Node 18 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 18 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx test @typescript-eslint/utils --coverage=false
• Node 14 - nx test @typescript-eslint/eslint-plugin-internal --coverage=false
• Node 14 - nx test @typescript-eslint/type-utils --coverage=false
• Node 14 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx test @typescript-eslint/scope-manager --coverage=false
• Node 14 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx test @typescript-eslint/visitor-keys --coverage=false
• Node 14 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx test @typescript-eslint/typescript-estree --coverage=false
• Node 14 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx test @typescript-eslint/parser --coverage=false
• Node 14 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx test @typescript-eslint/eslint-plugin-tslint --coverage=false
• Node 14 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx run-many --target=build --all --parallel --exclude website --exclude website
• nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx test @typescript-eslint/ast-spec --coverage=false
• Node 14 - nx run-many --target=build --all --parallel --exclude website --exclude website
• nx run-many --target=build --all --parallel --exclude website --exclude website

---

Sent with 💌 from NxCloud.

[typescript-eslint]

Thanks for the PR, @Andarist!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

---

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

[netlify]

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	cc2fc8c
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/628aaf6bb254e6000913f3e2
😎 Deploy Preview	https://deploy-preview-5036--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[JoshuaKGoldberg]

LGTM! If this solves your use case then 💯.

I suspect you could probably also remove the TRuleListeners from CLIEngine.ts and Rule.ts, but no worries if that's left for another day.

[codecov]

Codecov Report

Merging #5036 (fcf3f9d) into v6 (1f60fae) will increase coverage by 0.03%.
The diff coverage is 96.62%.

❗ Current head fcf3f9d differs from pull request most recent head f09d478. Consider uploading reports for the commit f09d478 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##               v6    #5036      +/-   ##
==========================================
+ Coverage   91.32%   91.36%   +0.03%     
==========================================
  Files         132      364     +232     
  Lines        1487    12134   +10647     
  Branches      224     3540    +3316     
==========================================
+ Hits         1358    11086    +9728     
- Misses         65      748     +683     
- Partials       64      300     +236     

Flag	Coverage Δ
unittest	91.36% <96.62%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
packages/eslint-plugin/src/rules/await-thenable.ts	100.00% <ø> (ø)
...ages/eslint-plugin/src/rules/ban-tslint-comment.ts	100.00% <ø> (ø)
...-plugin/src/rules/explicit-member-accessibility.ts	97.43% <ø> (ø)
...plugin/src/rules/explicit-module-boundary-types.ts	91.04% <ø> (ø)
...kages/eslint-plugin/src/rules/func-call-spacing.ts	96.87% <ø> (ø)
packages/eslint-plugin/src/rules/indent.ts	92.85% <ø> (ø)
...kages/eslint-plugin/src/rules/init-declarations.ts	100.00% <ø> (ø)
...ackages/eslint-plugin/src/rules/keyword-spacing.ts	100.00% <ø> (ø)
...nt-plugin/src/rules/lines-between-class-members.ts	92.85% <ø> (ø)
.../eslint-plugin/src/rules/member-delimiter-style.ts	94.73% <ø> (ø)
... and 308 more

[JoshuaKGoldberg]

NB, #5037 discusses when we should release the next major version. If anybody reading this feels strongly that this change shouldn't have to wait 5 months then please mention there. 🙂

[Andarist]

I suspect you could probably also remove the TRuleListeners from CLIEngine.ts and Rule.ts, but no worries if that's left for another day.

Done. I had to leave it in the RuleModule type in Rule.ts though (that's the core of why it's needed in the first place).

[JoshuaKGoldberg]

Just re-approving for paperwork purposes 🙂

[scamden]

NB, #5037 discusses when we should release the next major version. If anybody reading this feels strongly that this change shouldn't have to wait 5 months then please mention there. 🙂

I believe I'm running into this issue in a monorepo which exports it's internal plugin to a bunch of other packages that have tsconfig project references to the plugin package (and therefore require declaration : true for it). Its 5 months like in 2 days i think. Will this really be merged then? would love to have it asap. (Also let me know if there's another way to acheive the monorepo setup)

[bradzacher]

@scamden why do you have tsconfig references to the internal plugin? Plugins really shouldn't export any usable types or API because they are only consumed indirectly via the untyped ESLint config.

For a working example we have an internal plugin in this repo that works just fine.

[scamden]

we depend on the package in package.json as a pnpm workspace dep (so we can use the rules in each package's eslintrc). we also use an automated tool the creates the tsconfig refs based on any pnpm workspace dependencies: https://github.com/Bessonov/set-project-references

also we do in fact need the ref i think cause otherwise tsc --build won't know to rebuild the eslint-plugin package from those that depend on it to add to their eslintrc's, plz tell me i'm wrong :)

[scamden]

@bradzacher ping on the above. also any word on when this can be merged?

[scamden]

just another ping on this @bradzacher do you all compile using tsc --build option? i believe references are necessary for this to function

[JoshuaKGoldberg]

@scamden per our contributing guide, please don't comment on open PRs asking for updates. We're a volunteer team with limited resources or budget. This PR is on the list and we'll get to it when we can.

That being said, I started work on making a new v6 version (#5883) and hope to get this in soon, perhaps in the next week or so.

[scamden]

apologies! and thanks for your work! i was just answering this question

If anybody reading this feels strongly that this change shouldn't have to wait 5 months then please mention there. 🙂

as an anybody who saw a more immediate need. and i now see the "there" that i missed initially as well. my bad. thanks again!

[bradzacher]

@scamden

We do use project references and we do use --build, but we don't have any packages that depend on our plugins, so there is no need for the plugins to be compiled with declarations.

It seems like your workspace trades-off a lighter build without declarations in favour of automated configuration?

I definitely understand the desire to use that package to manage project references! It just seems like it's doing the wrong thing by making your production code build "depend on" your dev-only eslint plugins.

Generally I'd recommend just making the plugin build a pre-step for your lint command rather than doing the build implicitly via project references.

[scamden]

We do use project references and we do use --build, but we don't have any packages that depend on our plugins, so there is no need for the plugins to be compiled with declarations.

if you're using --build that builds referenced projects, if the eslint plugin is a referenced project it has to have composite : true which auto-enables declarations. if it's not a referenced project it it won't get built by --build, so maybe you are building it independently? point is if we want to use --build to auto build the things we have to build declarations whether dependents need them or not.

Either way, I realized I could solve this by explicitly declaring the return type to be ReturnType<typeof createRule> so we are unblocked. Thanks for the attention and for all the excellent work on this plugin!