fix(angular-query): do not run callbacks in injection context #8817
Conversation
|
View your CI Pipeline Execution ↗ for commit a0503de.
☁️ Nx Cloud last updated this comment at |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8817 +/- ##
===========================================
+ Coverage 46.45% 88.36% +41.91%
===========================================
Files 199 16 -183
Lines 7567 275 -7292
Branches 1740 44 -1696
===========================================
- Hits 3515 243 -3272
+ Misses 3672 31 -3641
+ Partials 380 1 -379
🚀 New features to boost your workflow:
|
|
@arnoud-dv This is not anty-pattern. It is normal angular code, where almost everything in Angular is based on DI. You can even pass injector to https://angular.dev/api/core/CreateEffectOptions 🚨 Rollback this PR it is breaking our codebase. If it were an anti-pattern, then https://angular.dev/api/core/runInInjectionContext would not exist. You can even see how NgRx Signal Store is allowing for that https://ngrx.io/guide/signals/signal-store#reactive-store-methods |
As the callback functions of
injectQueryandinjectMutationwere run in the injectionContext, usinginjectin these functions was possible. This is an anti-pattern in that it turns dependency injection into service locator. The Angular framework itself does not run callbacks in the injection context. Consider e.g. lifecycle hooks or callback functions on APIs such aseffectorcomputed.Running these callbacks in the injection context adds complexity to the adapter, and it wasn't consistent between callbacks, e.g. queryFn did not run in the injection context. Making it consistent by running other callbacks in the injection context too would require adding a lot more complexity.
To prevent the anti-pattern and to be consistent with the Angular framework and remove unnecessary complexity from the adapter the code changes in this PR will ensure callbacks are no longer run in the injection context.
@Component({ // .. }) export class SimpleExampleComponent { // ✅ inject on class contruction + private readonly http = inject(HttpClient) readonly query = injectQuery(() => { // ❌ injecting inside callback anti-pattern no longer possible - const http = inject(HttpClient) return { queryKey: ['repoData'], queryFn: () => lastValueFrom( - http.get<Response>('https://api.github.com/repos/tanstack/query'), + this.http.get<Response>('https://api.github.com/repos/tanstack/query'), ), } }) }Closes #8714