fix(express-engine): add bundleDependencies and lazy-loading fixes (#1167)

* fix(express-engine): add bundleDependencies and lazy-loading fixes

* fixup! fix(express-engine): add bundleDependencies and lazy-loading fixes

* fixup! fix(express-engine): add bundleDependencies and lazy-loading fixes

* fixup! fix(express-engine): add bundleDependencies and lazy-loading fixes

* fixup! fix(express-engine): add bundleDependencies and lazy-loading fixes

* fixup! fix(express-engine): add bundleDependencies and lazy-loading fixes

* fixup! fix(express-engine): add bundleDependencies and lazy-loading fixes

* fixup! fix(express-engine): add bundleDependencies and lazy-loading fixes

* fixup! fix(express-engine): add bundleDependencies and lazy-loading fixes

Author: CaerusKaru

.circleci/config.yml

@@ -12,7 +12,7 @@
 # fetched by the Webtesting rules. Therefore for jobs that run tests with Bazel, we don't need a
 # docker image with browsers pre-installed.
 # **NOTE**: If you change the the version of the docker images, also change the `cache_key` suffix.
-var_1: &docker_image angular/ngcontainer:0.10.0
+var_1: &docker_image circleci/node:10.12-browsers
 var_2: &cache_key v2-nguniversal-{{ .Branch }}-{{ checksum "yarn.lock" }}-node-10.12
 
 # Settings common to each job
@@ -80,8 +80,9 @@ jobs:
     - *checkout_code
     - *restore_cache
     - *copy_bazel_config
+    - *yarn_install
 
-    - run: bazel test //...
+    - run: yarn bazel test //...
 
     # Note: We want to save the cache in this job because the workspace cache also
     # includes the Bazel repository cache that will be updated in this job.

integration/express-engine/e2e/helloworld-spec.ts

@@ -42,5 +42,8 @@ describe('Hello world E2E Tests', function() {
     // Test the contents from the server.
     const serverDiv = browser.driver.findElement(by.css('span.href-check'));
     expect(serverDiv.getText()).toMatch('http://localhost:9876/helloworld');
+
+    // Make sure there were no client side errors.
+    verifyNoBrowserErrors();
    });
 });

integration/express-engine/src/server.ts

@@ -25,15 +25,14 @@ enableProdMode();
 app.use('/built', express.static('built'));
 
 // Keep the browser logs free of errors.
-app.get('/favicon.ico', (req, res) => { res.send(''); });
+app.get('/favicon.ico', (_, res) => { res.send(''); });
 
 //-----------ADD YOUR SERVER SIDE RENDERED APP HERE ----------------------
-app.get('/helloworld', (req: Request, res) =>
-  ngExpressEngine({bootstrap: HelloWorldServerModuleNgFactory})('built/src/index.html', {
+app.get('/helloworld', (req: Request, res) => ngExpressEngine({bootstrap: HelloWorldServerModuleNgFactory})('built/src/index.html', {
     bootstrap: HelloWorldServerModuleNgFactory,
     req,
     document: helloworld,
-  }, (err, html) => res.send(html))
+  }, (_, html) => res.send(html))
 );
 
 app.listen(9876, function() { console.log('Server listening on port 9876!'); });

integration/run_tests.sh

@@ -11,7 +11,7 @@ cd "$(dirname "$0")"
 readonly basedir=$(pwd)/..
 
 # Track payload size functions
-if [$CI != true]; then
+if !($CI); then
   # Not on CircleCI so let's build the packages-dist directory.
   # This should be fast on incremental re-build.
   ${basedir}/scripts/build-modules-dist.sh

modules/express-engine/schematics/BUILD.bazel

@@ -24,6 +24,7 @@ ts_library(
     module_name = "@nguniversal/express-engine/schematics",
     tsconfig = ":tsconfig.json",
     deps = [
+        "@npm//@angular-devkit/core",
         "@npm//@angular-devkit/schematics",
         "@npm//@schematics/angular",
         "@npm//@types/jasmine",
@@ -56,6 +57,7 @@ ng_test_library(
     tsconfig = ":tsconfig.json",
     deps = [
         ":schematics",
+        "@npm//@angular-devkit/core",
         "@npm//@angular-devkit/schematics",
         "@npm//@schematics/angular",
     ],

modules/express-engine/schematics/install/files/root/__serverFileName@stripTsExtension__.ts

@@ -1,24 +1,33 @@
+/**
+ * *** NOTE ON IMPORTING FROM ANGULAR AND NGUNIVERSAL IN THIS FILE ***
+ *
+ * If your application uses third-party dependencies, you'll need to
+ * either use Webpack or the Angular CLI's `bundleDependencies` feature
+ * in order to adequately package them for use on the server without a
+ * node_modules directory.
+ *
+ * However, due to the nature of the CLI's `bundleDependencies`, importing
+ * Angular in this file will create a different instance of Angular than
+ * the version in the compiled application code. This leads to unavoidable
+ * conflicts. Therefore, please do not explicitly import from @angular or
+ * @nguniversal in this file. You can export any needed resources
+ * from your application's main.server.ts file, as seen below with the
+ * import for `ngExpressEngine`.
+ */
+
 import 'zone.js/dist/zone-node';
-import {enableProdMode} from '@angular/core';
-// Express Engine
-import {ngExpressEngine} from '@nguniversal/express-engine';
-// Import module map for lazy loading
-import {provideModuleMap} from '@nguniversal/module-map-ngfactory-loader';
 
 import * as express from 'express';
 import {join} from 'path';
 
-// Faster server renders w/ Prod mode (dev mode never needed)
-enableProdMode();
-
 // Express server
 const app = express();
 
 const PORT = process.env.PORT || <%= serverPort %>;
 const DIST_FOLDER = join(process.cwd(), '<%= getBrowserDistDirectory() %>');
 
 // * NOTE :: leave this as require() since this file is built Dynamically from webpack
-const {AppServerModuleNgFactory, LAZY_MODULE_MAP} = require('./<%= getServerDistDirectory() %>/main');
+const {AppServerModuleNgFactory, LAZY_MODULE_MAP, ngExpressEngine, provideModuleMap} = require('./<%= getServerDistDirectory() %>/main');
 
 // Our Universal express-engine (found @ https://github.com/angular/universal/tree/master/modules/express-engine)
 app.engine('html', ngExpressEngine({

modules/express-engine/schematics/install/files/root/webpack.server.config.js

@@ -9,6 +9,9 @@ module.exports = {
     // This is our Express server for Dynamic universal
     server: './<%= stripTsExtension(serverFileName) %>.ts'
   },
+  externals: {
+    './<%= getServerDistDirectory() %>/main': 'require("./server/main")'
+  },
   target: 'node',
   resolve: { extensions: ['.ts', '.js'] },
   optimization: {
@@ -20,6 +23,7 @@ module.exports = {
     filename: '[name].js'
   },
   module: {
+    noParse: /polyfills-.*\.js/,
     rules: [
       { test: /\.ts$/, loader: 'ts-loader' },
       {

modules/express-engine/schematics/install/index.spec.ts

@@ -124,4 +124,15 @@ describe('Universal Schematic', () => {
     const contents = tree.readContent(filePath);
     expect(contents).toContain('ModuleMapLoaderModule');
   });
+
+  it('should add exports to main server file', async () => {
+    const tree = await schematicRunner
+      .runSchematicAsync('ng-add', defaultOptions, appTree)
+      .toPromise();
+    const filePath = '/projects/bar/src/main.server.ts';
+    const contents = tree.readContent(filePath);
+    console.log({contents});
+    expect(contents).toContain('ngExpressEngine');
+    expect(contents).toContain('provideModuleMap');
+  });
 });

modules/express-engine/schematics/install/index.ts

@@ -21,22 +21,24 @@ import {
   url,
 } from '@angular-devkit/schematics';
 import {NodePackageInstallTask} from '@angular-devkit/schematics/tasks';
-import {getWorkspace, getWorkspacePath} from '@schematics/angular/utility/config';
+import {getWorkspace} from '@schematics/angular/utility/config';
 import {Schema as UniversalOptions} from './schema';
 import {
   addPackageJsonDependency,
   NodeDependencyType,
 } from '@schematics/angular/utility/dependencies';
-import {BrowserBuilderOptions} from '@schematics/angular/utility/workspace-models';
 import {getProject} from '@schematics/angular/utility/project';
-import {
-  getProjectTargets,
-  targetBuildNotFoundError,
-} from '@schematics/angular/utility/project-targets';
+import {getProjectTargets} from '@schematics/angular/utility/project-targets';
 import {InsertChange} from '@schematics/angular/utility/change';
-import {addSymbolToNgModuleMetadata, insertImport} from '@schematics/angular/utility/ast-utils';
+import {
+  addSymbolToNgModuleMetadata,
+  findNodes,
+  insertAfterLastOccurrence,
+  insertImport
+} from '@schematics/angular/utility/ast-utils';
 import * as ts from 'typescript';
-import {findAppServerModulePath} from './utils';
+import {findAppServerModulePath, generateExport, getTsSourceFile, getTsSourceText} from './utils';
+import {updateWorkspace} from '@schematics/angular/utility/workspace';
 
 // TODO(CaerusKaru): make these configurable
 const BROWSER_DIST = 'dist/browser';
@@ -100,54 +102,50 @@ function addDependenciesAndScripts(options: UniversalOptions): Rule {
     pkg.scripts['serve:ssr'] = `node dist/${serverFileName}`;
     pkg.scripts['build:ssr'] = 'npm run build:client-and-server-bundles && npm run compile:server';
     pkg.scripts['build:client-and-server-bundles'] =
-      `ng build --prod && ng run ${options.clientProject}:server:production`;
+      // tslint:disable:max-line-length
+      `ng build --prod && ng run ${options.clientProject}:server:production --bundleDependencies all`;
 
     host.overwrite(pkgPath, JSON.stringify(pkg, null, 2));
 
     return host;
   };
 }
 
-function updateConfigFile(options: UniversalOptions): Rule {
-  return (host: Tree) => {
-    const workspace = getWorkspace(host);
-    if (!workspace.projects[options.clientProject]) {
-      throw new SchematicsException(`Client app ${options.clientProject} not found.`);
-    }
-
-    const clientProject = workspace.projects[options.clientProject];
-    if (!clientProject.architect) {
-      throw new Error('Client project architect not found.');
-    }
-
-    // We have to check if the project config has a server target, because
-    // if the Universal step in this schematic isn't run, it can't be guaranteed
-    // to exist
-    if (!clientProject.architect.server) {
-      return;
-    }
-
-    clientProject.architect.server.configurations = {
-      production: {
-        fileReplacements: [
-          {
-            replace: 'src/environments/environment.ts',
-            with: 'src/environments/environment.prod.ts'
-          }
-        ]
+function updateConfigFile(options: UniversalOptions) {
+  return updateWorkspace((workspace => {
+    const clientProject = workspace.projects.get(options.clientProject);
+    if (clientProject) {
+      const buildTarget = clientProject.targets.get('build');
+      const serverTarget = clientProject.targets.get('build');
+
+      // We have to check if the project config has a server target, because
+      // if the Universal step in this schematic isn't run, it can't be guaranteed
+      // to exist
+      if (!serverTarget || !buildTarget) {
+        return;
       }
-    };
-    // TODO(CaerusKaru): make this configurable
-    clientProject.architect.server.options.outputPath = SERVER_DIST;
-    // TODO(CaerusKaru): make this configurable
-    (clientProject.architect.build.options as BrowserBuilderOptions).outputPath = BROWSER_DIST;
-
-    const workspacePath = getWorkspacePath(host);
 
-    host.overwrite(workspacePath, JSON.stringify(workspace, null, 2));
-
-    return host;
-  };
+      serverTarget.configurations = {
+        production: {
+          fileReplacements: [
+            {
+              replace: 'src/environments/environment.ts',
+              with: 'src/environments/environment.prod.ts'
+            }
+          ]
+        }
+      };
+
+      serverTarget.options = {
+        ...serverTarget.options,
+        outputPath: SERVER_DIST,
+      };
+
+      buildTarget.options = {
+        outputPath: BROWSER_DIST,
+      };
+    }
+  }));
 }
 
 function addModuleMapLoader(options: UniversalOptions): Rule {
@@ -160,7 +158,6 @@ function addModuleMapLoader(options: UniversalOptions): Rule {
       return;
     }
     const mainPath = normalize('/' + clientTargets.server.options.main);
-
     const appServerModuleRelativePath = findAppServerModulePath(host, mainPath);
     const modulePath = normalize(
       `/${clientProject.root}/src/${appServerModuleRelativePath}.ts`);
@@ -192,15 +189,33 @@ function addModuleMapLoader(options: UniversalOptions): Rule {
   };
 }
 
-function getTsSourceFile(host: Tree, path: string): ts.SourceFile {
-  const buffer = host.read(path);
-  if (!buffer) {
-    throw new SchematicsException(`Could not read file (${path}).`);
-  }
-  const content = buffer.toString();
-  const source = ts.createSourceFile(path, content, ts.ScriptTarget.Latest, true);
+function addExports(options: UniversalOptions): Rule {
+  return (host: Tree) => {
+    const clientProject = getProject(host, options.clientProject);
+    const clientTargets = getProjectTargets(clientProject);
+
+    if (!clientTargets.server) {
+      // If they skipped Universal schematics and don't have a server target,
+      // just get out
+      return;
+    }
 
-  return source;
+    const mainPath = normalize('/' + clientTargets.server.options.main);
+    const mainSourceFile = getTsSourceFile(host, mainPath);
+    let mainText = getTsSourceText(host, mainPath);
+    const mainRecorder = host.beginUpdate(mainPath);
+    const expressEngineExport = generateExport(mainSourceFile, ['ngExpressEngine'],
+      '@nguniversal/express-engine');
+    const moduleMapExport = generateExport(mainSourceFile, ['provideModuleMap'],
+      '@nguniversal/module-map-ngfactory-loader');
+    const exports = findNodes(mainSourceFile, ts.SyntaxKind.ExportDeclaration);
+    const addedExports = `\n${expressEngineExport}\n${moduleMapExport}\n`;
+    const exportChange = insertAfterLastOccurrence(exports, addedExports, mainText,
+      0) as InsertChange;
+
+    mainRecorder.insertLeft(exportChange.pos, exportChange.toAdd);
+    host.commitUpdate(mainRecorder);
+  };
 }
 
 export default function (options: UniversalOptions): Rule {
@@ -234,6 +249,7 @@ export default function (options: UniversalOptions): Rule {
       mergeWith(rootSource),
       addDependenciesAndScripts(options),
       addModuleMapLoader(options),
+      addExports(options),
     ]);
   };
 }

modules/express-engine/schematics/install/utils.ts

@@ -9,15 +9,21 @@ import { SchematicsException, Tree } from '@angular-devkit/schematics';
 import * as ts from 'typescript';
 import {getSourceNodes} from '@schematics/angular/utility/ast-utils';
 
-export function findAppServerModuleExport(host: Tree,
-                                          mainPath: string): ts.ExportDeclaration | null {
-  const mainBuffer = host.read(mainPath);
-  if (!mainBuffer) {
-    throw new SchematicsException(`Main file (${mainPath}) not found`);
+export function getTsSourceText(host: Tree, path: string): string {
+  const buffer = host.read(path);
+  if (!buffer) {
+    throw new SchematicsException(`Could not read file (${path}).`);
   }
-  const mainText = mainBuffer.toString('utf-8');
-  const source = ts.createSourceFile(mainPath, mainText, ts.ScriptTarget.Latest, true);
+  return buffer.toString();
+}
+
+export function getTsSourceFile(host: Tree, path: string): ts.SourceFile {
+  return ts.createSourceFile(path, getTsSourceText(host, path), ts.ScriptTarget.Latest, true);
+}
 
+export function findAppServerModuleExport(host: Tree,
+                                          mainPath: string): ts.ExportDeclaration | null {
+  const source = getTsSourceFile(host, mainPath);
   const allNodes = getSourceNodes(source);
 
   let exportDeclaration: ts.ExportDeclaration | null = null;
@@ -49,6 +55,20 @@ export function findAppServerModulePath(host: Tree, mainPath: string): string {
     throw new SchematicsException('Could not find app server module export');
   }
 
-  const moduleSpecifier = exportDeclaration.moduleSpecifier.getText();
+  const moduleSpecifier = exportDeclaration.moduleSpecifier!.getText();
   return moduleSpecifier.substring(1, moduleSpecifier.length - 1);
 }
+
+export function generateExport(sourceFile: ts.SourceFile,
+                               elements: string[],
+                               module: string): string {
+  const printer = ts.createPrinter();
+  const exports = elements.map(element =>
+    ts.createExportSpecifier(undefined, element));
+  const namedExports = ts.createNamedExports(exports);
+  const moduleSpecifier = ts.createStringLiteral(module);
+  const exportDeclaration = ts.createExportDeclaration(undefined, undefined,
+    namedExports, moduleSpecifier);
+
+  return printer.printNode(ts.EmitHint.Unspecified, exportDeclaration, sourceFile);
+}