feat(eslint-plugin): [no-inject-outside-di-context] add rule#2892
feat(eslint-plugin): [no-inject-outside-di-context] add rule#2892cyrilletuzi wants to merge 12 commits into
Conversation
379892b to
a0bf84d
Compare
a0bf84d to
93015ad
Compare
|
@JamesHenry would it be possible to approve the workflow so I can check everything is OK? |
|
I'm not really a fan of such heuristics based rules because of the false positive/false negatives. |
However the injection context is not magical, there is a list of actual and finite places where the injection context is available. I have been careful to manage all these places, even the most advanced ones, to avoid false positives. I also have been careful to always do the checks in the most specific way possible, to avoid catching unrelated things and false negatives. What actual examples of false positives or false negatives do you see? Also, while I understand the general debate, other rules already exist and do the same thing (actually this one is based on #2803). |
|
Also, this rule is opt-in, so it does not enforce anything to anyone. |
The only real case where you can be sure you're not in an injection context is just after an I didn't like #2803, either because of the same said heuristics. |
Noted. So only false positives, and which are not common scenarios. It feels far better to me to have a lint rule, and to be a little over protected, as the risk to do an |
…ps://github.com/cyrilletuzi/angular-eslint into feat/eslint-plugin/no-inject-outside-di-context
|
For people interested, I published the rule (and other rules for similar functions) in a separate package: https://github.com/cyrilletuzi/angular-eslint-injection-context |
|
Sorry for the delay @cyrilletuzi let's stick with your community plugin for now, and we can potentially revisit in future, thanks a lot for your contribution |
Hello,
inject()is now the official recommended way to inject dependencies in Angular, and theprefer-injectrule is in the recommended set of Angular ESLint since v20.While this new syntax has been done for some reasons I will not expand here, there is one setback:
inject()must be used in an "injection context". There is 2 main issues with this:The 2 main errors developers do:
inject()inngOnInit()or other methodsinject()in a constructor or field initializer, but inside a callback where the injection context is lost (for example inside the callback of an Observable operator or a Promise)This rule aims to fix all these problems by checking
inject()is used only in allowed places.It was quite a challenge, but I think I manage to take into account all places where the injection context is available:
runInInjectionContext()provideAppInitializer(),providePlatformInitializer(),provideEnvironmentInitializer(), and view transitions options)assertInInjectionContext()await(which is equivalent to be in a.then()callback)This list was possible by the combination of:
About rule name and future-proof consistency: I hesitated between
no-inject-outside-di-contextandno-injection-outside-di-context. Indeed, this PR could easily be the base of other similar rules, as other functions likeeffect()ortoSignal()require an injection context. But note that the scenario is not exactly the same for 2 reasons:inject()does not)inject()requires an injection context by its own nature and purpose)So it would be more a generalization of #2803, and the future other rule could be called
no-implicit-injectororno-implicit-injection-context.Also, while I tried be as generic as possible, part of this rule code needed to be hard-coded (special functions names for example). While it will not change every day, I volunteer for maintaining this rule (my work allows me to keep a close eye on Angular releases and details).
Thanks to @JamesHenry for the amazing work, and to @rznn7 for #2803 which served as a model and base for this new rule.
Fixes #2300