@angular/pwa & SW related fixes/refactorings

packages/angular/pwa/pwa/index.ts

@@ -12,7 +12,6 @@ import {
   SchematicsException,
   Tree,
   apply,
-  branchAndMerge,
   chain,
   externalSchematic,
   mergeWith,
@@ -38,18 +37,8 @@ function addServiceWorker(options: PwaOptions): Rule {
 }
 
 function getIndent(text: string): string {
-  let indent = '';
-  let hitNonSpace = false;
-  text.split('')
-    .forEach(char => {
-      if (char === ' ' && !hitNonSpace) {
-        indent += ' ';
-      } else {
-        hitNonSpace = true;
-      }
-    }, 0);
-
-  return indent;
+  // This RegExp is guaranteed to match any string (even if it is a zero-length match).
+  return (/^ */.exec(text) as RegExpExecArray)[0];
 }
 
 function updateIndexFile(options: PwaOptions): Rule {
@@ -70,43 +59,32 @@ function updateIndexFile(options: PwaOptions): Rule {
     const content = buffer.toString();
     const lines = content.split('\n');
     let closingHeadTagLineIndex = -1;
-    let closingHeadTagLine = '';
     let closingBodyTagLineIndex = -1;
-    let closingBodyTagLine = '';
-    lines.forEach((line: string, index: number) => {
-      if (/<\/head>/.test(line) && closingHeadTagLineIndex === -1) {
-        closingHeadTagLine = line;
+    lines.forEach((line, index) => {
+      if (closingHeadTagLineIndex === -1 && /<\/head>/.test(line)) {
         closingHeadTagLineIndex = index;
-      }
-
-      if (/<\/body>/.test(line) && closingBodyTagLineIndex === -1) {
-        closingBodyTagLine = line;
+      } else if (closingBodyTagLineIndex === -1 && /<\/body>/.test(line)) {
         closingBodyTagLineIndex = index;
       }
     });
 
-    const headTagIndent = getIndent(closingHeadTagLine) + '  ';
+    const headIndent = getIndent(lines[closingHeadTagLineIndex]) + '  ';
     const itemsToAddToHead = [
       '<link rel="manifest" href="manifest.json">',
       '<meta name="theme-color" content="#1976d2">',
     ];
 
-    const textToInsertIntoHead = itemsToAddToHead
-      .map(text => headTagIndent + text)
-      .join('\n');
-
-    const bodyTagIndent = getIndent(closingBodyTagLine) + '  ';
-    const itemsToAddToBody
-      = '<noscript>Please enable JavaScript to continue using this application.</noscript>';
-
-    const textToInsertIntoBody = bodyTagIndent + itemsToAddToBody;
+    const bodyIndent = getIndent(lines[closingBodyTagLineIndex]) + '  ';
+    const itemsToAddToBody = [
+      '<noscript>Please enable JavaScript to continue using this application.</noscript>',
+    ];
 
     const updatedIndex = [
       ...lines.slice(0, closingHeadTagLineIndex),
-      textToInsertIntoHead,
+      ...itemsToAddToHead.map(line => headIndent + line),
       ...lines.slice(closingHeadTagLineIndex, closingBodyTagLineIndex),
-      textToInsertIntoBody,
-      ...lines.slice(closingBodyTagLineIndex),
+      ...itemsToAddToBody.map(line => bodyIndent + line),
+      ...lines.slice(closingHeadTagLineIndex),
     ].join('\n');
 
     host.overwrite(path, updatedIndex);
@@ -137,12 +115,9 @@ function addManifestToAssetsConfig(options: PwaOptions) {
     ['build', 'test'].forEach((target) => {
 
       const applyTo = architect[target].options;
+      const assets = applyTo.assets || (applyTo.assets = []);
 
-      if (!applyTo.assets) {
-        applyTo.assets = [assetEntry];
-      } else {
-        applyTo.assets.push(assetEntry);
-      }
+      assets.push(assetEntry);
 
     });
 
@@ -163,27 +138,24 @@ export default function (options: PwaOptions): Rule {
       throw new SchematicsException(`PWA requires a project type of "application".`);
     }
 
-    const assetPath = join(project.root as Path, 'src', 'assets');
     const sourcePath = join(project.root as Path, 'src');
+    const assetsPath = join(sourcePath, 'assets');
 
     options.title = options.title || options.project;
 
-    const templateSource = apply(url('./files/assets'), [
-      template({
-        ...options,
-      }),
-      move(assetPath),
+    const rootTemplateSource = apply(url('./files/root'), [
+      template({ ...options }),
+      move(sourcePath),
+    ]);
+    const assetsTemplateSource = apply(url('./files/assets'), [
+      template({ ...options }),
+      move(assetsPath),
     ]);
 
     return chain([
       addServiceWorker(options),
-      branchAndMerge(chain([
-        mergeWith(templateSource),
-      ])),
-      mergeWith(apply(url('./files/root'), [
-        template({...options}),
-        move(sourcePath),
-      ])),
+      mergeWith(rootTemplateSource),
+      mergeWith(assetsTemplateSource),
       updateIndexFile(options),
       addManifestToAssetsConfig(options),
     ])(host, context);

packages/schematics/angular/application/index.ts

@@ -23,6 +23,7 @@ import {
   url,
 } from '@angular-devkit/schematics';
 import { Schema as E2eOptions } from '../e2e/schema';
+import { PATH_TO_PACKAGE_JSON, addDependencies } from '../utility/add-dependencies';
 import {
   WorkspaceProject,
   WorkspaceSchema,
@@ -59,32 +60,21 @@ import { Schema as ApplicationOptions } from './schema';
 // }
 
 function addDependenciesToPackageJson() {
-  return (host: Tree) => {
-    const packageJsonPath = 'package.json';
+  const depsToAdd = [
+    `@angular-devkit/build-angular@${latestVersions.DevkitBuildAngular}`,
+    `@angular/compiler-cli@${latestVersions.Angular}`,
+    `typescript@${latestVersions.TypeScript}`,
+  ];
+  const rule = addDependencies(depsToAdd, true);
 
-    if (!host.exists('package.json')) { return host; }
-
-    const source = host.read('package.json');
-    if (!source) { return host; }
-
-    const sourceText = source.toString('utf-8');
-    const json = JSON.parse(sourceText);
-
-    if (!json['devDependencies']) {
-      json['devDependencies'] = {};
+  return (host: Tree, context: SchematicContext) => {
+    if (!host.exists(PATH_TO_PACKAGE_JSON)) {
+      // The default rule throws if `package.json` does not exist.
+      // Here, we want to be more lenient.
+      return host;
     }
 
-    json.devDependencies = {
-      '@angular/compiler-cli': latestVersions.Angular,
-      '@angular-devkit/build-angular': latestVersions.DevkitBuildAngular,
-      'typescript': latestVersions.TypeScript,
-      // De-structure last keeps existing user dependencies.
-      ...json.devDependencies,
-    };
-
-    host.overwrite(packageJsonPath, JSON.stringify(json, null, 2));
-
-    return host;
+    return rule(host, context);
   };
 }
 

packages/schematics/angular/application/index_spec.ts

@@ -139,39 +139,43 @@ describe('Application Schematic', () => {
   describe(`update package.json`, () => {
     it(`should add build-angular to devDependencies`, () => {
       const tree = schematicRunner.runSchematic('application', defaultOptions, workspaceTree);
+      const pkg = JSON.parse(tree.readContent('package.json'));
 
-      const packageJson = JSON.parse(tree.readContent('package.json'));
-      expect(packageJson.devDependencies['@angular-devkit/build-angular'])
+      expect(pkg.devDependencies['@angular-devkit/build-angular'])
         .toEqual(latestVersions.DevkitBuildAngular);
     });
 
     it('should use the latest known versions in package.json', () => {
       const tree = schematicRunner.runSchematic('application', defaultOptions, workspaceTree);
-      const pkg = JSON.parse(tree.readContent('/package.json'));
+      const pkg = JSON.parse(tree.readContent('package.json'));
+
       expect(pkg.devDependencies['@angular/compiler-cli']).toEqual(latestVersions.Angular);
       expect(pkg.devDependencies['typescript']).toEqual(latestVersions.TypeScript);
     });
 
     it(`should not override existing users dependencies`, () => {
-      const oldPackageJson = workspaceTree.readContent('package.json');
-      workspaceTree.overwrite('package.json', oldPackageJson.replace(
+      const oldPkg = workspaceTree.readContent('package.json');
+      workspaceTree.overwrite('package.json', oldPkg.replace(
         `"typescript": "${latestVersions.TypeScript}"`,
         `"typescript": "~2.5.2"`,
       ));
 
       const tree = schematicRunner.runSchematic('application', defaultOptions, workspaceTree);
-      const packageJson = JSON.parse(tree.readContent('package.json'));
-      expect(packageJson.devDependencies.typescript).toEqual('~2.5.2');
+      const pkg = JSON.parse(tree.readContent('package.json'));
+
+      expect(pkg.devDependencies.typescript).toEqual('~2.5.2');
     });
 
     it(`should not modify the file when --skipPackageJson`, () => {
-      const tree = schematicRunner.runSchematic('application', {
+      const options = {
         name: 'foo',
         skipPackageJson: true,
-      }, workspaceTree);
+      };
+
+      const tree = schematicRunner.runSchematic('application', options, workspaceTree);
+      const pkg = JSON.parse(tree.readContent('package.json'));
 
-      const packageJson = JSON.parse(tree.readContent('package.json'));
-      expect(packageJson.devDependencies['@angular-devkit/build-angular']).toBeUndefined();
+      expect(pkg.devDependencies['@angular-devkit/build-angular']).toBeUndefined();
     });
   });
 

packages/schematics/angular/service-worker/files/ngsw-config.json

@@ -1,24 +1,26 @@
 {
   "index": "/index.html",
-  "assetGroups": [{
-    "name": "app",
-    "installMode": "prefetch",
-    "resources": {
-      "files": [
-        "/favicon.ico",
-        "/index.html",
-        "/*.css",
-        "/*.js"
-      ]
+  "assetGroups": [
+    {
+      "name": "app",
+      "installMode": "prefetch",
+      "resources": {
+        "files": [
+          "/favicon.ico",
+          "/index.html",
+          "/*.css",
+          "/*.js"
+        ]
+      }
+    }, {
+      "name": "assets",
+      "installMode": "lazy",
+      "updateMode": "prefetch",
+      "resources": {
+        "files": [
+          "/assets/**"
+        ]
+      }
     }
-  }, {
-    "name": "assets",
-    "installMode": "lazy",
-    "updateMode": "prefetch",
-    "resources": {
-      "files": [
-        "/assets/**"
-      ]
-    }
-  }]
-}
+  ]
+}

packages/schematics/angular/service-worker/index.ts

@@ -10,7 +10,6 @@ import {
   SchematicContext,
   SchematicsException,
   Tree,
-  UpdateRecorder,
   apply,
   chain,
   mergeWith,
@@ -20,17 +19,13 @@ import {
 } from '@angular-devkit/schematics';
 import { NodePackageInstallTask } from '@angular-devkit/schematics/tasks';
 import * as ts from 'typescript';
+import { addDependencies } from '../utility/add-dependencies';
 import { addSymbolToNgModuleMetadata, insertImport, isImported } from '../utility/ast-utils';
 import { InsertChange } from '../utility/change';
-import {
-  getWorkspace,
-  getWorkspacePath,
-} from '../utility/config';
+import { getWorkspace, getWorkspacePath } from '../utility/config';
 import { getAppModulePath } from '../utility/ng-ast-utils';
 import { Schema as ServiceWorkerOptions } from './schema';
 
-const packageJsonPath = '/package.json';
-
 function updateConfigFile(options: ServiceWorkerOptions): Rule {
   return (host: Tree, context: SchematicContext) => {
     context.logger.debug('updating config file.');
@@ -68,26 +63,6 @@ function updateConfigFile(options: ServiceWorkerOptions): Rule {
   };
 }
 
-function addDependencies(): Rule {
-  return (host: Tree, context: SchematicContext) => {
-    const packageName = '@angular/service-worker';
-    context.logger.debug(`adding dependency (${packageName})`);
-    const buffer = host.read(packageJsonPath);
-    if (buffer === null) {
-      throw new SchematicsException('Could not find package.json');
-    }
-
-    const packageObject = JSON.parse(buffer.toString());
-
-    const ngCoreVersion = packageObject.dependencies['@angular/core'];
-    packageObject.dependencies[packageName] = ngCoreVersion;
-
-    host.overwrite(packageJsonPath, JSON.stringify(packageObject, null, 2));
-
-    return host;
-  };
-}
-
 function updateAppModule(options: ServiceWorkerOptions): Rule {
   return (host: Tree, context: SchematicContext) => {
     context.logger.debug('Updating appmodule');
@@ -134,7 +109,7 @@ function updateAppModule(options: ServiceWorkerOptions): Rule {
 
     // register SW in app module
     const importText =
-      `ServiceWorkerModule.register('/ngsw-worker.js', { enabled: environment.production })`;
+      `ServiceWorkerModule.register('ngsw-worker.js', { enabled: environment.production })`;
     moduleSource = getTsSourceFile(host, modulePath);
     const metadataChanges = addSymbolToNgModuleMetadata(
       moduleSource, modulePath, 'imports', importText);
@@ -185,7 +160,7 @@ export default function (options: ServiceWorkerOptions): Rule {
     return chain([
       mergeWith(templateSource),
       updateConfigFile(options),
-      addDependencies(),
+      addDependencies(['@angular/service-worker@{{NG_VERSION}}']),
       updateAppModule(options),
     ])(host, context);
   };

packages/schematics/angular/service-worker/index_spec.ts

@@ -87,8 +87,8 @@ describe('Service Worker Schematic', () => {
     const tree = schematicRunner.runSchematic('service-worker', defaultOptions, appTree);
     const pkgText = tree.readContent('/projects/bar/src/app/app.module.ts');
     // tslint:disable-next-line:max-line-length
-    const regex = /ServiceWorkerModule\.register\('\/ngsw-worker.js\', { enabled: environment.production }\)/;
-    expect(pkgText).toMatch(regex);
+    const expectedText = 'ServiceWorkerModule.register(\'ngsw-worker.js\', { enabled: environment.production })';
+    expect(pkgText).toContain(expectedText);
   });
 
   it('should put the ngsw-config.json file in the project root', () => {