From a33d02681aaa24eda4cbcc11a52ef4291c213051 Mon Sep 17 00:00:00 2001 From: Mark Whitfeld Date: Tue, 24 Nov 2020 15:27:05 +0200 Subject: [PATCH] fix(store): faulty select if requested before state added (#1701) * test: improve store tests layout * fix(store): faulty select if requested before state added fixes #1522 * chore: update bundle size * chore: remove unnecessary comment --- bundlesize.config.json | 4 +- packages/store/src/internal/state-factory.ts | 19 +- packages/store/tests/store.spec.ts | 190 +++++++++++++------ 3 files changed, 156 insertions(+), 57 deletions(-) diff --git a/bundlesize.config.json b/bundlesize.config.json index 6229122f6..1cdd00404 100644 --- a/bundlesize.config.json +++ b/bundlesize.config.json @@ -32,14 +32,14 @@ "path": "./@ngxs/store/fesm2015/ngxs-store.js", "package": "@ngxs/store", "target": "es2015", - "maxSize": "113.25KB", + "maxSize": "114.05KB", "compression": "none" }, { "path": "./@ngxs/store/fesm5/ngxs-store.js", "package": "@ngxs/store", "target": "es5", - "maxSize": "133.10KB", + "maxSize": "134.10KB", "compression": "none" }, { diff --git a/packages/store/src/internal/state-factory.ts b/packages/store/src/internal/state-factory.ts index 650ddcb73..5608f52b2 100644 --- a/packages/store/src/internal/state-factory.ts +++ b/packages/store/src/internal/state-factory.ts @@ -77,12 +77,27 @@ export class StateFactory implements OnDestroy { getRuntimeSelectorContext = memoize(() => { const stateFactory = this; + + function resolveGetter(key: string) { + const path = stateFactory.statePaths[key]; + return path ? propGetter(path.split('.'), stateFactory._config) : null; + } + const context: RuntimeSelectorContext = this._parentFactory ? this._parentFactory.getRuntimeSelectorContext() : { getStateGetter(key: string) { - const path = stateFactory.statePaths[key]; - return path ? propGetter(path.split('.'), stateFactory._config) : () => undefined; + let getter = resolveGetter(key); + if (getter) { + return getter; + } + return (...args) => { + // Late loaded getter + if (!getter) { + getter = resolveGetter(key); + } + return getter ? getter(...args) : undefined; + }; }, getSelectorOptions(localOptions?: SharedSelectorOptions) { const globalSelectorOptions = stateFactory._config.selectorOptions; diff --git a/packages/store/tests/store.spec.ts b/packages/store/tests/store.spec.ts index 1064de103..993a8e844 100644 --- a/packages/store/tests/store.spec.ts +++ b/packages/store/tests/store.spec.ts @@ -1,12 +1,13 @@ import { async, TestBed } from '@angular/core/testing'; -import { Observable } from 'rxjs'; +import { Observable, Subscription } from 'rxjs'; import { Store } from '../src/store'; import { NgxsModule } from '../src/module'; import { State } from '../src/decorators/state'; import { Action } from '../src/decorators/action'; import { StateContext } from '../src/symbols'; -import { Injectable } from '@angular/core'; +import { Injectable, ModuleWithProviders, NgModule, Type } from '@angular/core'; +import { tap } from 'rxjs/operators'; describe('Store', () => { interface SubSubStateModel { @@ -83,18 +84,30 @@ describe('Store', () => { @Injectable() class MyOtherState {} - let store: Store; - - beforeEach(() => { + function setup( + options: { + preImports?: (ModuleWithProviders | Type)[]; + } = {} + ) { TestBed.configureTestingModule({ - imports: [NgxsModule.forRoot([MySubState, MySubSubState, MyState, MyOtherState])] + imports: [ + ...(options.preImports || []), + NgxsModule.forRoot([MySubState, MySubSubState, MyState, MyOtherState]) + ] }); - store = TestBed.inject(Store); - }); + const store = TestBed.inject(Store); + return { + store + }; + } it('should subscribe to the root state', async(() => { + // Arrange + const { store } = setup(); + // Act store.subscribe((state: any) => { + // Assert expect(state).toEqual({ foo: { first: 'Hello', @@ -115,15 +128,119 @@ describe('Store', () => { })); it('should select the correct state use a function', async(() => { + // Arrange + const { store } = setup(); + // Act store .select((state: { foo: StateModel }) => state.foo.first) .subscribe(state => { + // Assert expect(state).toBe('Hello'); }); })); - it('should select the correct state use a state class: Root State', async(() => { - store.select(MyState).subscribe(state => { + describe('[select]', () => { + it('should select the correct state use a state class: Root State', async(() => { + // Arrange + const { store } = setup(); + // Act + store.select(MyState).subscribe(state => { + // Assert + expect(state).toEqual({ + first: 'Hello', + second: 'World', + bar: { + hello: true, + world: true, + baz: { + name: 'Danny' + } + } + }); + }); + })); + + it('should select the correct state use a state class: Sub State', async(() => { + // Arrange + const { store } = setup(); + // Act + // todo: remove any + store.select(MySubState).subscribe((state: SubStateModel) => { + // Assert + expect(state).toEqual({ + hello: true, + world: true, + baz: { + name: 'Danny' + } + }); + }); + })); + + it('should select the correct state use a state class: Sub Sub State', async(() => { + // Arrange + const { store } = setup(); + // Act + // todo: remove any + store + .select(MySubSubState) + .subscribe((state: SubSubStateModel) => { + // Assert + expect(state).toEqual({ + name: 'Danny' + }); + }); + })); + + it('should select state even when called before state added', async(() => { + // Arrange + @Injectable() + class CollectorService { + collected: string[] = []; + subscription: Subscription; + constructor(private store: Store) {} + + startCollecting() { + this.subscription = this.store + .select(MyState) + .pipe( + tap((model: StateModel) => { + this.collected.push(model?.first); + }) + ) + .subscribe(); + } + + stop() { + this.subscription?.unsubscribe(); + } + } + + @NgModule({ + providers: [CollectorService] + }) + class CollectorModule { + constructor(service: CollectorService) { + service.startCollecting(); + } + } + + // Act + setup({ preImports: [CollectorModule] }); + const collector = TestBed.inject(CollectorService); + collector.stop(); + // Assert + expect(collector.collected).toEqual([undefined, 'Hello']); + })); + }); + + describe('[selectSnapshot]', () => { + it('should select snapshot state use a state class', async(() => { + // Arrange + const { store } = setup(); + // Act + const state = store.selectSnapshot(MyState); + // Assert expect(state).toEqual({ first: 'Hello', second: 'World', @@ -135,52 +252,19 @@ describe('Store', () => { } } }); - }); - })); - - it('should select the correct state use a state class: Sub State', async(() => { - // todo: remove any - store.select(MySubState).subscribe((state: SubStateModel) => { - expect(state).toEqual({ - hello: true, - world: true, - baz: { - name: 'Danny' - } - }); - }); - })); + })); - it('should select the correct state use a state class: Sub Sub State', async(() => { - // todo: remove any - store.select(MySubSubState).subscribe((state: SubSubStateModel) => { + it('should select state with an underscore in name', async(() => { + // Arrange + const { store } = setup(); + // Act + const state = store.selectSnapshot(MyOtherState); + // Assert expect(state).toEqual({ - name: 'Danny' + under: 'score' }); - }); - })); - - it('should select snapshot state use a state class', async(() => { - const state = store.selectSnapshot(MyState); - expect(state).toEqual({ - first: 'Hello', - second: 'World', - bar: { - hello: true, - world: true, - baz: { - name: 'Danny' - } - } - }); - })); - - it('should select state with an underscore in name', async(() => { - const state = store.selectSnapshot(MyOtherState); - expect(state).toEqual({ - under: 'score' - }); - })); + })); + }); // it('should not require you to subscrube in order to dispatch', () => {}); });