From 456efc0c3fc84f3858bab08062a84dfa5e5173a4 Mon Sep 17 00:00:00 2001 From: arturovt Date: Tue, 19 Nov 2024 18:45:53 +0200 Subject: [PATCH] perf(store): prevent initializing state factory at feature levels In this commit, we mark the state factory as a root provider to prevent its initialization at feature levels when `provideStates` is called. Previously, the state factory was initialized at feature levels because it needed to retrieve the feature-level injector in order to call `injector.get(FeatureStateClass)`. This was necessary because, when `NgxsModule.forFeature([BlogState])` was called, the `BlogState` instance could only be retrieved from the feature injector hierarchy. Now, since `addAndReturnDefaults` is called at the feature level inside the feature environment initializer, it is executed within the feature injection context, allowing us to use the `inject(...)` function directly. --- CHANGELOG.md | 1 + packages/store/src/internal/state-factory.ts | 91 +++++++------------ .../standalone-features/feature-providers.ts | 2 - .../src/standalone-features/root-providers.ts | 2 - 4 files changed, 35 insertions(+), 61 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index abbc39b9d..3adc6a9af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ $ npm install @ngxs/store@dev - Fix(websocket-plugin): Do not dispatch action when root injector is destroyed [#2257](https://github.com/ngxs/store/pull/2257) - Refactor(store): Replace `exhaustMap` [#2254](https://github.com/ngxs/store/pull/2254) - Refactor(store): Tree-shake development options token [#2260](https://github.com/ngxs/store/pull/2260) +- Performance(store): Prevent initializing state factory at feature levels [#2261](https://github.com/ngxs/store/pull/2261) ### 18.1.5 2024-11-12 diff --git a/packages/store/src/internal/state-factory.ts b/packages/store/src/internal/state-factory.ts index f8bbb8153..7da192221 100644 --- a/packages/store/src/internal/state-factory.ts +++ b/packages/store/src/internal/state-factory.ts @@ -69,20 +69,17 @@ function cloneDefaults(defaults: any): any { * The `StateFactory` class adds root and feature states to the graph. * This extracts state names from state classes, checks if they already * exist in the global graph, throws errors if their names are invalid, etc. - * See its constructor, state factories inject state factories that are - * parent-level providers. This is required to get feature states from the - * injector on the same level. * - * The `NgxsModule.forFeature(...)` returns `providers: [StateFactory, ...states]`. - * The `StateFactory` is initialized on the feature level and goes through `...states` - * to get them from the injector through `injector.get(state)`. + * Root and feature initializers call `addAndReturnDefaults()` to add those states + * to the global graph. Since `addAndReturnDefaults` runs within the injection + * context (which might be the root injector or a feature injector), we can + * retrieve an instance of the state class using `inject(StateClass)`. * @ignore */ -@Injectable() +@Injectable({ providedIn: 'root' }) export class StateFactory implements OnDestroy { private readonly _injector = inject(Injector); private readonly _config = inject(NgxsConfig); - private readonly _parentFactory = inject(StateFactory, { optional: true, skipSelf: true }); private readonly _stateContextFactory = inject(StateContextFactory); private readonly _actions = inject(InternalActions); private readonly _actionResults = inject(InternalDispatchedActionResults); @@ -95,19 +92,8 @@ export class StateFactory implements OnDestroy { private _ngxsUnhandledErrorHandler: NgxsUnhandledErrorHandler = null!; private _states: MappedStore[] = []; - get states(): MappedStore[] { - return this._parentFactory ? this._parentFactory.states : this._states; - } - private _statesByName: StatesByName = {}; - get statesByName(): StatesByName { - return this._parentFactory ? this._parentFactory.statesByName : this._statesByName; - } - private _statePaths: ɵPlainObjectOf = {}; - private get statePaths(): ɵPlainObjectOf { - return this._parentFactory ? this._parentFactory.statePaths : this._statePaths; - } getRuntimeSelectorContext = ɵmemoize(() => { // eslint-disable-next-line @typescript-eslint/no-this-alias @@ -115,36 +101,34 @@ export class StateFactory implements OnDestroy { const propGetter = stateFactory._propGetter; function resolveGetter(key: string) { - const path = stateFactory.statePaths[key]; + const path = stateFactory._statePaths[key]; return path ? propGetter(path.split('.')) : null; } - const context: ɵRuntimeSelectorContext = this._parentFactory - ? this._parentFactory.getRuntimeSelectorContext() - : { - getStateGetter(key: string) { - // Use `@__INLINE__` annotation to forcely inline `resolveGetter`. - // This is a Terser annotation, which will function only in the production mode. - let getter = /*@__INLINE__*/ resolveGetter(key); - if (getter) { - return getter; - } - return (...args) => { - // Late loaded getter - if (!getter) { - getter = /*@__INLINE__*/ resolveGetter(key); - } - return getter ? getter(...args) : undefined; - }; - }, - getSelectorOptions(localOptions?: ɵSharedSelectorOptions) { - const globalSelectorOptions = stateFactory._config.selectorOptions; - return { - ...globalSelectorOptions, - ...(localOptions || {}) - }; + const context: ɵRuntimeSelectorContext = { + getStateGetter(key: string) { + // Use `@__INLINE__` annotation to forcely inline `resolveGetter`. + // This is a Terser annotation, which will function only in the production mode. + let getter = /*@__INLINE__*/ resolveGetter(key); + if (getter) { + return getter; + } + return (...args) => { + // Late loaded getter + if (!getter) { + getter = /*@__INLINE__*/ resolveGetter(key); } + return getter ? getter(...args) : undefined; + }; + }, + getSelectorOptions(localOptions?: ɵSharedSelectorOptions) { + const globalSelectorOptions = stateFactory._config.selectorOptions; + return { + ...globalSelectorOptions, + ...(localOptions || {}) }; + } + }; return context; }); @@ -155,7 +139,7 @@ export class StateFactory implements OnDestroy { /** * Add a new state to the global defs. */ - add(stateClasses: ɵStateClassInternal[]): MappedStore[] { + private add(stateClasses: ɵStateClassInternal[]): MappedStore[] { if (typeof ngDevMode !== 'undefined' && ngDevMode) { ensureStatesAreDecorated(stateClasses); } @@ -189,7 +173,7 @@ export class StateFactory implements OnDestroy { path, isInitialised: false, actions: meta.actions, - instance: this._injector.get(stateClass), + instance: inject(stateClass), defaults: cloneDefaults(meta.defaults) }; @@ -200,7 +184,7 @@ export class StateFactory implements OnDestroy { bootstrappedStores.push(stateMap); } - this.states.push(stateMap); + this._states.push(stateMap); this.hydrateActionMetasMap(stateMap); } @@ -223,13 +207,6 @@ export class StateFactory implements OnDestroy { } connectActionHandlers(): void { - // Note: We have to connect actions only once when the `StateFactory` - // is being created for the first time. This checks if we're in - // a child state factory and the parent state factory already exists. - if (this._parentFactory || this._actionsSubscription !== null) { - return; - } - this._actionsSubscription = this._actions .pipe( filter((ctx: ActionContext) => ctx.status === ActionStatus.Dispatched), @@ -306,7 +283,7 @@ export class StateFactory implements OnDestroy { newStates: ɵStateClassInternal[]; } { const newStates: ɵStateClassInternal[] = []; - const statesMap: StatesByName = this.statesByName; + const statesMap: StatesByName = this._statesByName; for (const stateClass of stateClasses) { const stateName = ɵgetStoreMetadata(stateClass).name!; @@ -324,7 +301,7 @@ export class StateFactory implements OnDestroy { } private addRuntimeInfoToMeta(meta: ɵMetaDataModel, path: string): void { - this.statePaths[meta.name!] = path; + this._statePaths[meta.name!] = path; // TODO: versions after v3 - we plan to get rid of the `path` property because it is non-deterministic // we can do this when we get rid of the incorrectly exposed getStoreMetadata // We will need to come up with an alternative to what was exposed in v3 because this is used by many plugins @@ -336,7 +313,7 @@ export class StateFactory implements OnDestroy { getValue(this._initialState, path) !== undefined; // This checks whether a state has been already added to the global graph and // its lifecycle is in 'bootstrapped' state. - return this.statesByName[name] && valueIsBootstrappedInInitialState; + return this._statesByName[name] && valueIsBootstrappedInInitialState; } private hydrateActionMetasMap({ path, actions, instance }: MappedStore): void { diff --git a/packages/store/src/standalone-features/feature-providers.ts b/packages/store/src/standalone-features/feature-providers.ts index c115bfaa9..35c6469b1 100644 --- a/packages/store/src/standalone-features/feature-providers.ts +++ b/packages/store/src/standalone-features/feature-providers.ts @@ -3,7 +3,6 @@ import { ɵStateClass } from '@ngxs/store/internals'; import { FEATURE_STATE_TOKEN } from '../symbols'; import { PluginManager } from '../plugin-manager'; -import { StateFactory } from '../internal/state-factory'; /** * This function provides the required providers when calling `NgxsModule.forFeature` @@ -11,7 +10,6 @@ import { StateFactory } from '../internal/state-factory'; */ export function getFeatureProviders(states: ɵStateClass[]): Provider[] { return [ - StateFactory, PluginManager, ...states, { diff --git a/packages/store/src/standalone-features/root-providers.ts b/packages/store/src/standalone-features/root-providers.ts index 73c50e530..a5cb86a7f 100644 --- a/packages/store/src/standalone-features/root-providers.ts +++ b/packages/store/src/standalone-features/root-providers.ts @@ -1,7 +1,6 @@ import { APP_BOOTSTRAP_LISTENER, Provider, inject } from '@angular/core'; import { ɵStateClass, ɵNgxsAppBootstrappedState } from '@ngxs/store/internals'; -import { StateFactory } from '../internal/state-factory'; import { CUSTOM_NGXS_EXECUTION_STRATEGY } from '../execution/symbols'; import { NgxsModuleOptions, ROOT_STATE_TOKEN, NGXS_OPTIONS } from '../symbols'; @@ -14,7 +13,6 @@ export function getRootProviders( options: NgxsModuleOptions ): Provider[] { return [ - StateFactory, ...states, { provide: ROOT_STATE_TOKEN,