From 14021c07857c9542ae17eb6b7f8502d83b98fd50 Mon Sep 17 00:00:00 2001 From: Artur Date: Mon, 18 Nov 2024 14:10:32 +0000 Subject: [PATCH] fix(store): run plugins in injection context (#2256) In this commit, we enable plugin functions to run within the injection context. This allows plugin functions (but not classes) to call `inject` and access dependencies. Additionally, we've moved `compose` to the `Dispatcher` class since it was only used once. Keeping it in the `Dispatcher` ensures it isn't used elsewhere and can be safely inlined if needed. --- CHANGELOG.md | 1 + packages/store/src/internal/dispatcher.ts | 36 ++++++++++++-- packages/store/src/utils/compose.ts | 29 ------------ packages/store/tests/plugins.spec.ts | 58 ++++++++++++++++------- 4 files changed, 75 insertions(+), 49 deletions(-) delete mode 100644 packages/store/src/utils/compose.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 1836787e0..84b7743db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ $ npm install @ngxs/store@dev ### To become next patch version - Refactor: Use field initializers for injectees [#2258](https://github.com/ngxs/store/pull/2258) +- Fix(store): Run plugins in injection context [#2256](https://github.com/ngxs/store/pull/2256) - Fix(websocket-plugin): Do not dispatch action when root injector is destroyed [#2257](https://github.com/ngxs/store/pull/2257) ### 18.1.5 2024-11-12 diff --git a/packages/store/src/internal/dispatcher.ts b/packages/store/src/internal/dispatcher.ts index 0866333bf..364d7a225 100644 --- a/packages/store/src/internal/dispatcher.ts +++ b/packages/store/src/internal/dispatcher.ts @@ -1,11 +1,10 @@ -import { inject, Injectable, NgZone } from '@angular/core'; +import { inject, Injectable, Injector, NgZone, runInInjectionContext } from '@angular/core'; import { EMPTY, forkJoin, Observable, of, Subject, throwError } from 'rxjs'; import { exhaustMap, filter, map, shareReplay, take } from 'rxjs/operators'; import { getActionTypeFromInstance } from '@ngxs/store/plugins'; import { ɵPlainObject, ɵStateStream } from '@ngxs/store/internals'; -import { compose } from '../utils/compose'; import { ActionContext, ActionStatus, InternalActions } from '../actions-stream'; import { PluginManager } from '../plugin-manager'; import { InternalNgxsExecutionStrategy } from '../execution/internal-ngxs-execution-strategy'; @@ -29,6 +28,7 @@ export class InternalDispatcher { private _pluginManager = inject(PluginManager); private _stateStream = inject(ɵStateStream); private _ngxsExecutionStrategy = inject(InternalNgxsExecutionStrategy); + private _injector = inject(Injector); /** * Dispatches event(s). @@ -70,7 +70,7 @@ export class InternalDispatcher { const prevState = this._stateStream.getValue(); const plugins = this._pluginManager.plugins; - return compose([ + return compose(this._injector, [ ...plugins, (nextState: any, nextAction: any) => { if (nextState !== prevState) { @@ -115,3 +115,33 @@ export class InternalDispatcher { .pipe(shareReplay()); } } + +type StateFn = (...args: any[]) => any; + +/** + * Composes a array of functions from left to right. Example: + * + * compose([fn, final])(state, action); + * + * then the funcs have a signature like: + * + * function fn (state, action, next) { + * console.log('here', state, action, next); + * return next(state, action); + * } + * + * function final (state, action) { + * console.log('here', state, action); + * return state; + * } + * + * the last function should not call `next`. + */ +const compose = + (injector: Injector, funcs: StateFn[]) => + (...args: any[]) => { + const curr = funcs.shift()!; + return runInInjectionContext(injector, () => + curr(...args, (...nextArgs: any[]) => compose(injector, funcs)(...nextArgs)) + ); + }; diff --git a/packages/store/src/utils/compose.ts b/packages/store/src/utils/compose.ts deleted file mode 100644 index 2c7337f49..000000000 --- a/packages/store/src/utils/compose.ts +++ /dev/null @@ -1,29 +0,0 @@ -export type StateFn = (...args: any[]) => any; - -/** - * Composes a array of functions from left to right. Example: - * - * compose([fn, final])(state, action); - * - * then the funcs have a signature like: - * - * function fn (state, action, next) { - * console.log('here', state, action, next); - * return next(state, action); - * } - * - * function final (state, action) { - * console.log('here', state, action); - * return state; - * } - * - * the last function should not call `next`. - * - * @ignore - */ -export const compose = - (funcs: StateFn[]) => - (...args: any[]) => { - const curr = funcs.shift()!; - return curr(...args, (...nextArgs: any[]) => compose(funcs)(...nextArgs)); - }; diff --git a/packages/store/tests/plugins.spec.ts b/packages/store/tests/plugins.spec.ts index 4655b4d7c..b288da8f9 100644 --- a/packages/store/tests/plugins.spec.ts +++ b/packages/store/tests/plugins.spec.ts @@ -1,48 +1,72 @@ +import { assertInInjectionContext } from '@angular/core'; import { TestBed } from '@angular/core/testing'; -import { NgxsModule, NGXS_PLUGINS, Store } from '@ngxs/store'; -import { tap } from 'rxjs/operators'; -import { Observable } from 'rxjs'; +import { NgxsModule, NGXS_PLUGINS, Store, NgxsNextPluginFn, InitState } from '@ngxs/store'; +import { debounceTime, firstValueFrom, tap } from 'rxjs'; describe('Plugins', () => { - it('should run a function plugin', () => { - let pluginInvoked = 0; + it('should run a function plugin (within an injection context too)', async () => { + // Arrange + const recorder: any[] = []; class Foo { static readonly type = 'Foo'; } - function logPlugin( - state: any, - action: any, - next: (state: any, action: any) => Observable - ) { - if (action.constructor && action.constructor.type === 'Foo') { - pluginInvoked++; + function asyncLogPlugin(state: any, action: any, next: NgxsNextPluginFn) { + assertInInjectionContext(asyncLogPlugin); + + if (action.constructor.type === 'Foo') { + recorder.push(['asyncLogPlugin()', action, 'before next()']); } return next(state, action).pipe( + debounceTime(0), tap(() => { if (action.constructor.type === 'Foo') { - pluginInvoked++; + recorder.push(['asyncLogPlugin()', action, 'after next()']); } }) ); } + function otherPlugin(state: any, action: any, next: NgxsNextPluginFn) { + assertInInjectionContext(otherPlugin); + recorder.push(['otherPlugin()', action]); + return next(state, action); + } + TestBed.configureTestingModule({ imports: [NgxsModule.forRoot()], providers: [ { provide: NGXS_PLUGINS, - useValue: logPlugin, + useValue: asyncLogPlugin, + multi: true + }, + { + provide: NGXS_PLUGINS, + useValue: otherPlugin, multi: true } ] }); - const store: Store = TestBed.inject(Store); - store.dispatch(new Foo()); + // Act + const store = TestBed.inject(Store); + + // Assert + expect(recorder).toEqual([['otherPlugin()', new InitState()]]); + + // Act + const action = new Foo(); + await firstValueFrom(store.dispatch(action)); - expect(pluginInvoked).toEqual(2); + // Assert + expect(recorder).toEqual([ + ['otherPlugin()', new InitState()], + ['asyncLogPlugin()', action, 'before next()'], + ['otherPlugin()', action], + ['asyncLogPlugin()', action, 'after next()'] + ]); }); });