refactor: major clean up of all engines

- add return type to domContentLoadedFactory
- remove unused DI tokens
- Remove FileLoader implementations from all engines. Instead use the one in @nguniversal/common
- Extend RenderOptions Express and Hapi engines interfaces from Common Engine RenderOptions

Author: alan-agius4

modules/aspnetcore-engine/BUILD.bazel

@@ -11,6 +11,7 @@ ng_module(
     module_name = "@nguniversal/aspnetcore-engine",
     deps = [
         "//modules/aspnetcore-engine/tokens",
+        "//modules/common/engine",
         "@npm//@angular/common",
         "@npm//@angular/compiler",
         "@npm//@angular/platform-browser",

modules/aspnetcore-engine/package.json

@@ -25,6 +25,7 @@
   "peerDependencies": {
     "@angular/common": "NG_VERSION",
     "@angular/core": "NG_VERSION",
+    "@nguniversal/common": "0.0.0-PLACEHOLDER",
     "rxjs": "RXJS_VERSION"
   },
   "ng-update": {

modules/aspnetcore-engine/src/file-loader.ts

@@ -11,86 +11,58 @@ import { Compiler, CompilerFactory, NgModuleFactory, StaticProvider, Type } from
 import { platformDynamicServer } from '@angular/platform-server';
 
 import { ORIGIN_URL, REQUEST } from '@nguniversal/aspnetcore-engine/tokens';
-import { FileLoader } from './file-loader';
+import { ɵFileLoader } from '@nguniversal/common/engine';
 import { IEngineOptions } from './interfaces/engine-options';
 import { IEngineRenderResult } from './interfaces/engine-render-result';
 import { renderModuleFactory } from './platform-server-utils';
 
-/* @internal */
-export class UniversalData {
-  appNode = '';
-  title = '';
-  scripts = '';
-  styles = '';
-  meta = '';
-  links = '';
-}
-
 /* @internal */
 let appSelector = 'app-root'; // default
 
 /* @internal */
-function _getUniversalData(doc: Document): UniversalData {
+function _getUniversalData(doc: Document): Omit<IEngineRenderResult, 'moduleRef'> {
 
-  const STYLES: string[] = [];
-  const SCRIPTS: string[] = [];
-  const META: string[] = [];
-  const LINKS: string[] = [];
+  const styles: string[] = [];
+  const scripts: string[] = [];
+  const meta: string[] = [];
+  const links: string[] = [];
 
-  // tslint:disable-next-line: no-non-null-assertion
-  for (let i = 0; i < doc.head!.children.length; i++) {
-    // tslint:disable-next-line: no-non-null-assertion
-    const element = doc.head!.children[i];
-    const tagName = element.tagName.toUpperCase();
-
-    switch (tagName) {
-      case 'SCRIPT':
-        SCRIPTS.push(element.outerHTML);
-        break;
-      case 'STYLE':
-        STYLES.push(element.outerHTML);
-        break;
-      case 'LINK':
-        LINKS.push(element.outerHTML);
-        break;
-      case 'META':
-        META.push(element.outerHTML);
-        break;
-      default:
-        break;
-    }
-  }
-
-  for (let i = 0; i < doc.body.children.length; i++) {
-    const element: Element = doc.body.children[i];
-    const tagName = element.tagName.toUpperCase();
+  // tslint:disable: no-non-null-assertion
+  const elements = [
+    ...Array.from(doc.head!.children),
+    ...Array.from(doc.body!.children),
+  ];
+  // tslint:enable: no-non-null-assertion
 
-    switch (tagName) {
+  for (const element of elements) {
+    switch (element.tagName.toUpperCase()) {
       case 'SCRIPT':
-        SCRIPTS.push(element.outerHTML);
+        scripts.push(element.outerHTML);
         break;
       case 'STYLE':
-        STYLES.push(element.outerHTML);
+        styles.push(element.outerHTML);
         break;
       case 'LINK':
-        LINKS.push(element.outerHTML);
+        links.push(element.outerHTML);
         break;
       case 'META':
-        META.push(element.outerHTML);
+        meta.push(element.outerHTML);
         break;
       default:
         break;
     }
   }
 
   return {
-    title: doc.title,
     // tslint:disable-next-line: no-non-null-assertion
-    appNode: doc.querySelector(appSelector)!.outerHTML,
-    scripts: SCRIPTS.join('\n'),
-    styles: STYLES.join('\n'),
-    meta: META.join('\n'),
-    links: LINKS.join('\n')
+    html: doc.querySelector(appSelector)!.outerHTML,
+    globals: {
+      title: doc.title,
+      scripts: scripts.join('\n'),
+      styles: styles.join('\n'),
+      meta: meta.join('\n'),
+      links: links.join('\n')
+    }
   };
 }
 
@@ -109,7 +81,7 @@ export async function ngAspnetCoreEngine(options: Readonly<IEngineOptions>)
   const compiler: Compiler = compilerFactory.createCompiler([
     {
       providers: [
-        { provide: ResourceLoader, useClass: FileLoader, deps: [] }
+        { provide: ResourceLoader, useClass: ɵFileLoader, deps: [] }
       ]
     }
   ]);
@@ -132,18 +104,10 @@ export async function ngAspnetCoreEngine(options: Readonly<IEngineOptions>)
   });
 
   const doc = result.moduleRef.injector.get(DOCUMENT);
-  const universalData = _getUniversalData(doc);
 
   return {
-    html: universalData.appNode,
     moduleRef: result.moduleRef,
-    globals: {
-      styles: universalData.styles,
-      title: universalData.title,
-      scripts: universalData.scripts,
-      meta: universalData.meta,
-      links: universalData.links
-    }
+    ..._getUniversalData(doc),
   };
 }
 

modules/aspnetcore-engine/src/main.ts

@@ -44,7 +44,8 @@ describe('Prerender Builder Utils', () => {
       loggerErrorSpy = spyOn(CONTEXT.logger, 'error');
     });
 
-    it('Should return the union of the routes from routes, routesFile, and the extracted routes without any parameterized routes', async () => {
+    it('Should return the union of the routes from routes, routesFile, and the extracted routes without any parameterized routes',
+    async () => {
       const options = {
         routes: ROUTES,
         routesFile: ROUTES_FILE,

modules/builders/src/prerender/utils.spec.ts

@@ -11,7 +11,6 @@ ng_module(
     module_name = "@nguniversal/common",
     deps = [
         "//modules/common/engine",
-        "//modules/common/tokens",
         "@npm//@angular/common",
         "@npm//@angular/core",
         "@npm//@angular/platform-browser",
@@ -29,7 +28,6 @@ ng_package(
     deps = [
         ":common",
         "//modules/common/engine",
-        "//modules/common/tokens",
     ],
 )
 

modules/common/BUILD.bazel

@@ -6,5 +6,8 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-export { FileLoader as ɵFileLoader, CommonEngine as ɵCommonEngine,
-  RenderOptions as ɵRenderOptions } from './src/index';
+export {
+  FileLoader as ɵFileLoader,
+  CommonEngine as ɵCommonEngine,
+  RenderOptions as ɵRenderOptions,
+} from './src/index';

modules/common/engine/private_api.ts

@@ -8,10 +8,18 @@
 import { ResourceLoader } from '@angular/compiler';
 import { Compiler, CompilerFactory, NgModuleFactory, StaticProvider, Type } from '@angular/core';
 import { INITIAL_CONFIG, platformDynamicServer, renderModuleFactory } from '@angular/platform-server';
-import * as fs from 'fs';
 
 import { FileLoader } from './file-loader';
-import { RenderOptions } from './interfaces';
+import { readFile } from './utils';
+
+/** These are the allowed options for the render */
+export interface RenderOptions {
+  bootstrap: Type<{}> | NgModuleFactory<{}>;
+  providers?: StaticProvider[];
+  url?: string;
+  document?: string;
+  documentFilePath?: string;
+}
 
 /**
  * A common rendering engine utility. This abstracts the logic
@@ -30,7 +38,7 @@ export class CommonEngine {
   }
 
   private factoryCacheMap = new Map<Type<{}>, NgModuleFactory<{}>>();
-  private templateCache: {[key: string]: string} = {};
+  private templateCache = new Map<string, string>();
 
   constructor(private moduleOrFactory?: Type<{}> | NgModuleFactory<{}>,
               private providers: StaticProvider[] = []) {}
@@ -41,7 +49,7 @@ export class CommonEngine {
    */
   async render(opts: RenderOptions): Promise<string> {
     // if opts.document dosen't exist then opts.documentFilePath must
-    const doc = opts.document || await this.getDocument(opts.documentFilePath as string);
+    const doc = opts.document || opts.documentFilePath && await this.getDocument(opts.documentFilePath);
     const extraProviders = [
       ...(opts.providers || []),
       ...(this.providers || []),
@@ -83,11 +91,14 @@ export class CommonEngine {
   }
 
   /** Retrieve the document from the cache or the filesystem */
-  private getDocument(filePath: string): Promise<string> {
-    const doc = this.templateCache[filePath] = this.templateCache[filePath] ||
-    fs.readFileSync(filePath).toString();
+  async getDocument(filePath: string): Promise<string> {
+    let doc = this.templateCache.get(filePath);
+
+    if (!doc) {
+      doc = await readFile(filePath, 'utf-8');
+      this.templateCache.set(filePath, doc);
+    }
 
-    // As  promise so we can change the API later without breaking
-    return Promise.resolve(doc);
+    return doc;
   }
 }

modules/common/engine/src/engine.ts

@@ -6,19 +6,11 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import { ResourceLoader } from '@angular/compiler';
-import * as fs from 'fs';
+import { readFile } from './utils';
 
 /** ResourceLoader implementation for loading files */
 export class FileLoader implements ResourceLoader {
   get(url: string): Promise<string> {
-    return new Promise((resolve, reject) => {
-      fs.readFile(url, (err: NodeJS.ErrnoException | null, data: Buffer) => {
-        if (err) {
-          return reject(err);
-        }
-
-        resolve(data.toString());
-      });
-    });
+    return readFile(url, 'utf-8');
   }
 }

modules/common/engine/src/file-loader.ts

@@ -5,6 +5,5 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
-export * from './interfaces';
 export * from './file-loader';
 export * from './engine';

modules/common/engine/src/index.ts

@@ -5,4 +5,8 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
-export * from './private_api';
+
+import * as fs from 'fs';
+import { promisify } from 'util';
+
+export const readFile = promisify(fs.readFile);

modules/common/engine/src/interfaces.ts

@@ -9,17 +9,21 @@
 import { DOCUMENT } from '@angular/common';
 import { APP_INITIALIZER, NgModule } from '@angular/core';
 
-export function domContentLoadedFactory(doc: Document) {
+
+export function domContentLoadedFactory(doc: Document): () => Promise<void>  {
   return () => new Promise ((resolve, _reject) => {
+    if (doc.readyState === 'complete' || doc.readyState === 'interactive') {
+      resolve();
+
+      return;
+    }
+
     const contentLoaded = () => {
       doc.removeEventListener('DOMContentLoaded', contentLoaded);
       resolve();
     };
-    if (doc.readyState === 'complete' || doc.readyState === 'interactive') {
-      resolve();
-    } else {
-      doc.addEventListener('DOMContentLoaded', contentLoaded);
-    }
+
+    doc.addEventListener('DOMContentLoaded', contentLoaded);
   });
 }