fix(store): faulty select if requested before state added (ngxs#1701)

* test: improve store tests layout * fix(store): faulty select if requested before state added fixes ngxs#1522 * chore: update bundle size * chore: remove unnecessary comment
felix-ulonska · Nov 24, 2020 · a33d026 · a33d026
1 parent 5a679a3
commit a33d026
Show file tree

Hide file tree

Showing 3 changed files with 156 additions and 57 deletions.
diff --git 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
@@ -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
@@ -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<any> | Type<any>)[];
+    } = {}
+  ) {
     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<SubStateModel>(<any>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<SubSubStateModel>(<any>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<SubStateModel>(<any>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<SubSubStateModel>(<any>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', () => {});
 });