From e4765d6fe1e3a721584c0b2e16d7d7883fb7dbf8 Mon Sep 17 00:00:00 2001
From: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com>
Date: Mon, 14 Oct 2019 04:24:12 +0200
Subject: [PATCH] fix(store): improve createFeatureSelector warning (#2163)

Closes #2116
---
 modules/store/spec/selector.spec.ts | 69 +++++++++++++++--------------
 modules/store/src/selector.ts       |  4 +-
 2 files changed, 37 insertions(+), 36 deletions(-)

diff --git a/modules/store/spec/selector.spec.ts b/modules/store/spec/selector.spec.ts
index f1198ae677..ef79e474b0 100644
--- a/modules/store/spec/selector.spec.ts
+++ b/modules/store/spec/selector.spec.ts
@@ -455,7 +455,7 @@ describe('Selectors', () => {
   });
 
   describe('createFeatureSelector', () => {
-    let featureName = '@ngrx/router-store';
+    let featureName = 'featureA';
     let featureSelector: (state: any) => number;
     let warnSpy: jasmine.Spy;
 
@@ -481,50 +481,51 @@ describe('Selectors', () => {
       );
 
       expect(featureState$).toBeObservable(expected$);
-      expect(warnSpy).not.toHaveBeenCalled();
     });
 
-    it('should warn if the feature does not exist in the state', () => {
-      spyOn(ngCore, 'isDevMode').and.returnValue(true);
+    describe('Warning', () => {
+      describe('should not log when: ', () => {
+        it('the feature does exist', () => {
+          spyOn(ngCore, 'isDevMode').and.returnValue(true);
+          const selector = createFeatureSelector('featureA');
 
-      const state = { otherState: '' };
+          selector({ featureA: {} });
 
-      const state$ = cold('a', { a: state });
-      const expected$ = cold('a', { a: undefined });
+          expect(warnSpy).not.toHaveBeenCalled();
+        });
 
-      const featureState$ = state$.pipe(
-        map(featureSelector),
-        distinctUntilChanged()
-      );
+        it('the feature key exist but is falsy', () => {
+          spyOn(ngCore, 'isDevMode').and.returnValue(true);
+          const selector = createFeatureSelector('featureB');
 
-      expect(featureState$).toBeObservable(expected$);
-      expect(warnSpy).toHaveBeenCalledWith(
-        'The feature name "@ngrx/router-store" does not exist ' +
-          'in the state, therefore createFeatureSelector cannot ' +
-          'access it.  Be sure it is imported in a loaded module using ' +
-          "StoreModule.forRoot('@ngrx/router-store', ...) or " +
-          "StoreModule.forFeature('@ngrx/router-store', ...).  If the " +
-          'default state is intended to be undefined, as is the case ' +
-          'with router state, this development-only warning message can ' +
-          'be ignored.'
-      );
-    });
+          selector({ featureA: {}, featureB: undefined });
 
-    it('should not warn if not in development mode', () => {
-      spyOn(ngCore, 'isDevMode').and.returnValue(false);
+          expect(warnSpy).not.toHaveBeenCalled();
+        });
 
-      const state = { otherState: '' };
+        it('not in development mode', () => {
+          spyOn(ngCore, 'isDevMode').and.returnValue(false);
+          const selector = createFeatureSelector('featureB');
 
-      const state$ = cold('a', { a: state });
-      const expected$ = cold('a', { a: undefined });
+          selector({ featureA: {} });
 
-      const featureState$ = state$.pipe(
-        map(featureSelector),
-        distinctUntilChanged()
-      );
+          expect(warnSpy).not.toHaveBeenCalled();
+        });
+      });
 
-      expect(featureState$).toBeObservable(expected$);
-      expect(warnSpy).not.toHaveBeenCalled();
+      describe('should log when: ', () => {
+        it('feature key does not exist', () => {
+          spyOn(ngCore, 'isDevMode').and.returnValue(true);
+          const selector = createFeatureSelector('featureB');
+
+          selector({ featureA: {} });
+
+          expect(warnSpy).toHaveBeenCalled();
+          expect(warnSpy.calls.mostRecent().args[0]).toMatch(
+            /The feature name "featureB" does not exist/
+          );
+        });
+      });
     });
   });
 
diff --git a/modules/store/src/selector.ts b/modules/store/src/selector.ts
index 5cffc55426..db444b395b 100644
--- a/modules/store/src/selector.ts
+++ b/modules/store/src/selector.ts
@@ -605,9 +605,9 @@ export function createFeatureSelector(
 ): MemoizedSelector<any, any> {
   return createSelector((state: any) => {
     const featureState = state[featureName];
-    if (isDevMode() && featureState === undefined) {
+    if (isDevMode() && !(featureName in state)) {
       console.warn(
-        `The feature name \"${featureName}\" does ` +
+        `@ngrx/store: The feature name \"${featureName}\" does ` +
           'not exist in the state, therefore createFeatureSelector ' +
           'cannot access it.  Be sure it is imported in a loaded module ' +
           `using StoreModule.forRoot('${featureName}', ...) or ` +