fix(deep-linking): fix bug with null deep link config when modifying the main NgModule file (app.module.ts)

Author: danbucholtz

src/deep-linking.spec.ts

@@ -14,7 +14,7 @@ describe('Deep Linking task', () => {
       deepLinking.reset();
     });
 
-    it('should not update app ngmodule when it has an existing deeplink config', () => {
+   it('should not update app ngmodule when it has an existing deeplink config', () => {
       const appNgModulePath = join('some', 'fake', 'path', 'myApp', 'src', 'app', 'app.module.ts');
       const context = {
         fileCache: new FileCache()
@@ -192,5 +192,148 @@ describe('Deep Linking task', () => {
         expect(spy).toHaveBeenCalledTimes(1);
       });
     });
+
+    it('should update the deeplink config and cached deeplink string no matter what when the app.module.ts is changed', () => {
+      const appNgModulePath = join('some', 'fake', 'path', 'myApp', 'src', 'app', 'app.module.ts');
+      const context = {
+        fileCache: new FileCache(),
+        runAot: true
+      };
+      const knownFileContent = `
+import { BrowserModule } from '@angular/platform-browser';
+import { HttpModule } from '@angular/http';
+import { NgModule, ErrorHandler } from '@angular/core';
+
+import { IonicApp, IonicModule, IonicErrorHandler } from 'ionic-angular';
+
+import { InAppBrowser } from '@ionic-native/in-app-browser';
+import { SplashScreen } from '@ionic-native/splash-screen';
+
+import { IonicStorageModule } from '@ionic/storage';
+
+import { ConferenceApp } from './app.component';
+
+import { ConferenceData } from '../providers/conference-data';
+import { UserData } from '../providers/user-data';
+
+@NgModule({
+  declarations: [
+    ConferenceApp
+  ],
+  imports: [
+    BrowserModule,
+    HttpModule,
+    IonicModule.forRoot(ConferenceApp, {
+      preloadModules: true
+    }),
+    IonicStorageModule.forRoot()
+  ],
+  bootstrap: [IonicApp],
+  entryComponents: [
+    ConferenceApp
+  ],
+  providers: [
+    { provide: ErrorHandler, useClass: IonicErrorHandler },
+    ConferenceData,
+    UserData,
+    InAppBrowser,
+    SplashScreen
+  ]
+})
+export class AppModule { }
+`;
+
+      const knownFileContent2 = `
+import { BrowserModule } from '@angular/platform-browser';
+import { HttpModule } from '@angular/http';
+import { NgModule, ErrorHandler } from '@angular/core';
+
+import { IonicApp, IonicModule, IonicErrorHandler } from 'ionic-angular';
+
+import { InAppBrowser } from '@ionic-native/in-app-browser';
+import { SplashScreen } from '@ionic-native/splash-screen';
+
+import { IonicStorageModule } from '@ionic/storage';
+
+import { ConferenceApp } from './app.component';
+
+import { ConferenceData } from '../providers/conference-data';
+import { UserData } from '../providers/user-data';
+
+@NgModule({
+  declarations: [
+    ConferenceApp,
+    SomeNewComponent
+  ],
+  imports: [
+    BrowserModule,
+    HttpModule,
+    IonicModule.forRoot(ConferenceApp, {
+      preloadModules: true
+    }),
+    IonicStorageModule.forRoot()
+  ],
+  bootstrap: [IonicApp],
+  entryComponents: [
+    ConferenceApp
+  ],
+  providers: [
+    { provide: ErrorHandler, useClass: IonicErrorHandler },
+    ConferenceData,
+    UserData,
+    InAppBrowser,
+    SplashScreen
+  ]
+})
+export class AppModule { }
+`;
+      const knownDeepLinkString = 'someDeepLinkString';
+      const knownMockDeepLinkArray = [1];
+      const changedFiles: ChangedFile[] = [];
+      context.fileCache.set(appNgModulePath, { path: appNgModulePath, content: knownFileContent});
+
+      spyOn(helpers, helpers.getStringPropertyValue.name).and.returnValue(appNgModulePath);
+      spyOn(deeplinkUtils, deeplinkUtils.getDeepLinkData.name).and.returnValue(knownMockDeepLinkArray);
+      spyOn(deeplinkUtils, deeplinkUtils.hasExistingDeepLinkConfig.name).and.returnValue(false);
+
+      spyOn(deeplinkUtils, deeplinkUtils.convertDeepLinkConfigEntriesToString.name).and.returnValue(knownDeepLinkString);
+
+      const spy = spyOn(deeplinkUtils, deeplinkUtils.updateAppNgModuleAndFactoryWithDeepLinkConfig.name);
+
+      const promise = deepLinking.deepLinkingWorkerImpl(context, changedFiles);
+
+      return promise.then(() => {
+        expect(deepLinking.cachedUnmodifiedAppNgModuleFileContent).toEqual(knownFileContent);
+        expect(deepLinking.cachedDeepLinkString).toEqual(knownDeepLinkString);
+        expect(helpers.getStringPropertyValue).toBeCalledWith(Constants.ENV_APP_NG_MODULE_PATH);
+        expect(deeplinkUtils.getDeepLinkData).toHaveBeenCalledWith(appNgModulePath, context.fileCache, context.runAot);
+        expect(deeplinkUtils.hasExistingDeepLinkConfig).toHaveBeenCalledWith(appNgModulePath, knownFileContent);
+        expect(deeplinkUtils.convertDeepLinkConfigEntriesToString).toHaveBeenCalledWith(knownMockDeepLinkArray);
+        expect(spy.calls.first().args[0]).toEqual(context);
+        expect(spy.calls.first().args[1]).toEqual(knownDeepLinkString);
+        expect(spy.calls.first().args[2]).toEqual(changedFiles);
+        expect(spy.calls.first().args[3]).toEqual(context.runAot);
+
+        // add a changed file to the fray
+        changedFiles.push({
+          event: 'change',
+          ext: '.ts',
+          filePath: appNgModulePath
+        });
+        context.fileCache.set(appNgModulePath, { path: appNgModulePath, content: knownFileContent2});
+        return deepLinking.deepLinkingWorkerImpl(context, changedFiles);
+      }).then((result) => {
+        expect(result).toEqual(knownMockDeepLinkArray);
+        expect(deepLinking.cachedDeepLinkString).toEqual(knownDeepLinkString);
+        expect(deepLinking.cachedUnmodifiedAppNgModuleFileContent).toEqual(knownFileContent2);
+        expect(deeplinkUtils.getDeepLinkData).toHaveBeenCalledTimes(2);
+        expect(deeplinkUtils.getDeepLinkData).toHaveBeenCalledWith(appNgModulePath, context.fileCache, context.runAot);
+        expect(deeplinkUtils.hasExistingDeepLinkConfig).toHaveBeenCalledTimes(2);
+        expect(deeplinkUtils.hasExistingDeepLinkConfig).toHaveBeenCalledWith(appNgModulePath, knownFileContent);
+        expect(deeplinkUtils.convertDeepLinkConfigEntriesToString).toHaveBeenCalledWith(knownMockDeepLinkArray);
+        expect(deeplinkUtils.convertDeepLinkConfigEntriesToString).toHaveBeenCalledTimes(2);
+        expect(spy).toHaveBeenCalledTimes(2);
+      });
+    });
   });
 });

src/deep-linking.ts

@@ -33,14 +33,14 @@ export function deepLinking(context: BuildContext) {
 
 
 function deepLinkingWorker(context: BuildContext) {
-  return deepLinkingWorkerImpl(context, null);
+  return deepLinkingWorkerImpl(context, []);
 }
 
 export function deepLinkingWorkerImpl(context: BuildContext, changedFiles: ChangedFile[]) {
   return Promise.resolve().then(() => {
     const appNgModulePath = getStringPropertyValue(Constants.ENV_APP_NG_MODULE_PATH);
     const appNgModuleFile = context.fileCache.get(appNgModulePath);
-    if (!cachedUnmodifiedAppNgModuleFileContent) {
+    if (!cachedUnmodifiedAppNgModuleFileContent || hasAppModuleChanged(changedFiles, appNgModulePath)) {
       cachedUnmodifiedAppNgModuleFileContent = appNgModuleFile.content;
     }
 
@@ -53,13 +53,11 @@ export function deepLinkingWorkerImpl(context: BuildContext, changedFiles: Chang
     const deepLinkConfigEntries = getDeepLinkData(appNgModulePath, context.fileCache, context.runAot) || [];
     if (deepLinkConfigEntries.length) {
       const newDeepLinkString = convertDeepLinkConfigEntriesToString(deepLinkConfigEntries);
-      if (!cachedDeepLinkString) {
-        // this is the first time running this, so update the build either way
-        cachedDeepLinkString = newDeepLinkString;
-        updateAppNgModuleAndFactoryWithDeepLinkConfig(context, newDeepLinkString, changedFiles, context.runAot);
-      } else if (newDeepLinkString !== cachedDeepLinkString) {
-        // we have an existing deep link string, and we have a new one, and they're different
-        // so go ahead and update the config
+
+      // 1. this is the first time running this, so update the build either way
+      // 2. we have an existing deep link string, and we have a new one, and they're different - so go ahead and update the config
+      // 3. the app's main ngmodule has changed, so we need to rewrite the config
+      if (!cachedDeepLinkString || newDeepLinkString !== cachedDeepLinkString || hasAppModuleChanged(changedFiles, appNgModulePath)) {
         cachedDeepLinkString = newDeepLinkString;
         updateAppNgModuleAndFactoryWithDeepLinkConfig(context, newDeepLinkString, changedFiles, context.runAot);
       }
@@ -68,6 +66,18 @@ export function deepLinkingWorkerImpl(context: BuildContext, changedFiles: Chang
   });
 }
 
+export function hasAppModuleChanged(changedFiles: ChangedFile[], appNgModulePath: string) {
+  if (!changedFiles) {
+    changedFiles = [];
+  }
+  for (const changedFile of changedFiles) {
+    if (changedFile.filePath === appNgModulePath) {
+      return true;
+    }
+  }
+  return false;
+}
+
 export function deepLinkingUpdate(changedFiles: ChangedFile[], context: BuildContext) {
   if (context.deepLinkState === BuildState.RequiresBuild) {
     return deepLinkingWorkerFullUpdate(context);
@@ -109,4 +119,4 @@ export function deepLinkingWorkerFullUpdate(context: BuildContext) {
 export function reset() {
   cachedUnmodifiedAppNgModuleFileContent = null;
   cachedDeepLinkString = null;
-}
+}