fix(source-maps): always generate source map, then purge them if not needed in postprocess step

Author: danbucholtz

config/rollup.config.js

@@ -19,7 +19,7 @@ var rollupConfig = {
    * sourceMap: If true, a separate sourcemap file will
    * be created.
    */
-  sourceMap: process.env.IONIC_GENERATE_SOURCE_MAP === 'true' ? true : false,
+  sourceMap: true,
 
   /**
    * format: The format of the generated bundle

config/webpack.config.js

@@ -10,7 +10,7 @@ module.exports = {
     filename: '[name].js',
     devtoolModuleFilenameTemplate: ionicWebpackFactory.getSourceMapperFunction(),
   },
-  devtool: process.env.IONIC_GENERATE_SOURCE_MAP === 'true' ? process.env.IONIC_SOURCE_MAP_TYPE : '',
+  devtool: process.env.IONIC_SOURCE_MAP_TYPE,
 
   resolve: {
     extensions: ['.ts', '.js', '.json'],

src/build.spec.ts

@@ -9,6 +9,7 @@ import * as clean  from './clean';
 import * as lint from './lint';
 import * as minify from './minify';
 import * as ngc from './ngc';
+import * as postprocess from './postprocess';
 import * as preprocess from './preprocess';
 import * as sass from './sass';
 import * as transpile from './transpile';
@@ -26,15 +27,16 @@ describe('build', () => {
     });
 
 
-    spyOn(bundle, 'bundle').and.returnValue(Promise.resolve());
-    spyOn(copy, 'copy').and.returnValue(Promise.resolve());
-    spyOn(minify, 'minifyCss').and.returnValue(Promise.resolve());
-    spyOn(minify, 'minifyJs').and.returnValue(Promise.resolve());
-    spyOn(lint, 'lint').and.returnValue(Promise.resolve());
-    spyOn(ngc, 'ngc').and.returnValue(Promise.resolve());
-    spyOn(preprocess, 'preprocess').and.returnValue(Promise.resolve());
-    spyOn(sass, 'sass').and.returnValue(Promise.resolve());
-    spyOn(transpile, 'transpile').and.returnValue(Promise.resolve());
+    spyOn(bundle, bundle.bundle.name).and.returnValue(Promise.resolve());
+    spyOn(copy, copy.copy.name).and.returnValue(Promise.resolve());
+    spyOn(minify, minify.minifyCss.name).and.returnValue(Promise.resolve());
+    spyOn(minify, minify.minifyJs.name).and.returnValue(Promise.resolve());
+    spyOn(lint, lint.lint.name).and.returnValue(Promise.resolve());
+    spyOn(ngc, ngc.ngc.name).and.returnValue(Promise.resolve());
+    spyOn(postprocess, postprocess.postprocess.name).and.returnValue(Promise.resolve());
+    spyOn(preprocess, preprocess.preprocess.name).and.returnValue(Promise.resolve());
+    spyOn(sass, sass.sass.name).and.returnValue(Promise.resolve());
+    spyOn(transpile, transpile.transpile.name).and.returnValue(Promise.resolve());
   });
 
   it('should do a prod build', () => {
@@ -79,6 +81,7 @@ describe('build', () => {
       expect(bundle.bundle).toHaveBeenCalled();
       expect(sass.sass).toHaveBeenCalled();
       expect(lint.lint).toHaveBeenCalled();
+      expect(postprocess.postprocess).toHaveBeenCalled();
       expect(preprocess.preprocess).toHaveBeenCalled();
       expect(ngc.ngc).not.toHaveBeenCalled();
       expect(minify.minifyJs).not.toHaveBeenCalled();
@@ -162,17 +165,18 @@ describe('test project requirements before building', () => {
     process.env[Constants.ENV_APP_ENTRY_POINT] = 'src/app/main.ts';
     process.env[Constants.ENV_TS_CONFIG] = 'tsConfig.js';
 
-    spyOn(bundle, 'bundle').and.returnValue(Promise.resolve());
-    spyOn(clean, 'clean');
-    spyOn(copy, 'copy').and.returnValue(Promise.resolve());
-    spyOn(minify, 'minifyCss').and.returnValue(Promise.resolve());
-    spyOn(minify, 'minifyJs').and.returnValue(Promise.resolve());
-    spyOn(lint, 'lint').and.returnValue(Promise.resolve());
-    spyOn(ngc, 'ngc').and.returnValue(Promise.resolve());
-    spyOn(preprocess, 'preprocess').and.returnValue(Promise.resolve());
-    spyOn(sass, 'sass').and.returnValue(Promise.resolve());
-    spyOn(transpile, 'transpile').and.returnValue(Promise.resolve());
-    spyOn(helpers, 'readFileAsync').and.callFake(() => {
+    spyOn(bundle, bundle.bundle.name).and.returnValue(Promise.resolve());
+    spyOn(clean, clean.clean.name);
+    spyOn(copy, copy.copy.name).and.returnValue(Promise.resolve());
+    spyOn(minify, minify.minifyCss.name).and.returnValue(Promise.resolve());
+    spyOn(minify, minify.minifyJs.name).and.returnValue(Promise.resolve());
+    spyOn(lint, lint.lint.name).and.returnValue(Promise.resolve());
+    spyOn(ngc, ngc.ngc.name).and.returnValue(Promise.resolve());
+    spyOn(postprocess, postprocess.postprocess.name).and.returnValue(Promise.resolve());
+    spyOn(preprocess, preprocess.preprocess.name).and.returnValue(Promise.resolve());
+    spyOn(sass, sass.sass.name).and.returnValue(Promise.resolve());
+    spyOn(transpile, transpile.transpile.name).and.returnValue(Promise.resolve());
+    spyOn(helpers, helpers.readFileAsync.name).and.callFake(() => {
       return Promise.resolve(`{
         "compilerOptions": {
           "sourceMap": true

src/build.ts

@@ -2,14 +2,15 @@ import * as Constants from './util/constants';
 import { BuildContext, BuildState, BuildUpdateMessage, ChangedFile } from './util/interfaces';
 import { BuildError } from './util/errors';
 import { emit, EventType } from './util/events';
-import { readFileAsync, setContext } from './util/helpers';
+import { getBooleanPropertyValue, readFileAsync, setContext } from './util/helpers';
 import { bundle, bundleUpdate } from './bundle';
 import { clean } from './clean';
 import { copy } from './copy';
 import { lint, lintUpdate } from './lint';
 import { Logger } from './logger/logger';
 import { minifyCss, minifyJs } from './minify';
 import { ngc } from './ngc';
+import { postprocess } from './postprocess';
 import { preprocess, preprocessUpdate } from './preprocess';
 import { sass, sassUpdate } from './sass';
 import { templateUpdate } from './template';
@@ -42,6 +43,8 @@ function buildWorker(context: BuildContext) {
     return preprocess(context);
   }).then(() => {
     return buildProject(context);
+  }).then(() => {
+    return postprocess(context);
   });
 }
 
@@ -122,8 +125,11 @@ function buildProject(context: BuildContext) {
     })
     .then(() => {
       // kick off the tslint after everything else
-      // nothing needs to wait on its completion
-      return lint(context);
+      // nothing needs to wait on its completion unless bailing on lint error is enabled
+      const result = lint(context);
+      if (getBooleanPropertyValue(Constants.ENV_BAIL_ON_LINT_ERROR)) {
+        return result;
+      }
     })
     .catch(err => {
       throw new BuildError(err);

src/postprocess.ts

@@ -0,0 +1,19 @@
+import { Logger } from './logger/logger';
+import { BuildContext } from './util/interfaces';
+import { purgeSourceMapsIfNeeded } from './util/source-maps';
+
+
+export function postprocess(context: BuildContext) {
+  const logger = new Logger(`postprocess`);
+  return postprocessWorker(context).then(() => {
+      logger.finish();
+    })
+    .catch((err: Error) => {
+      throw logger.fail(err);
+    });
+}
+
+
+function postprocessWorker(context: BuildContext) {
+  return purgeSourceMapsIfNeeded(context);
+}

src/uglifyjs.ts

@@ -1,11 +1,14 @@
-import { BuildContext, TaskInfo } from './util/interfaces';
-import { BuildError } from './util/errors';
-import { fillConfigDefaults, generateContext, getUserConfigFile } from './util/config';
 import { join } from 'path';
+import * as uglify from 'uglify-js';
+
 import { Logger } from './logger/logger';
+import { fillConfigDefaults, generateContext, getUserConfigFile } from './util/config';
+import * as Constants from './util/constants';
+import { BuildError } from './util/errors';
+import { getBooleanPropertyValue, writeFileAsync } from './util/helpers';
+import { BuildContext, TaskInfo } from './util/interfaces';
 import { runWorker } from './worker-client';
-import { writeFileAsync } from './util/helpers';
-import * as uglify from 'uglify-js';
+
 
 
 export function uglifyjs(context: BuildContext, configFile?: string) {

src/util/helpers.spec.ts

@@ -0,0 +1,42 @@
+import * as helpers from './helpers';
+
+describe('helpers', () => {
+  describe('getBooleanPropertyValue', () => {
+    it('should return true when value is "true"', () => {
+      // arrange
+      const propertyName = 'test';
+      const propertyValue = 'true';
+      let environment: any = { };
+      environment[propertyName] = propertyValue;
+      process.env = environment;
+      // act
+      const result = helpers.getBooleanPropertyValue(propertyName);
+      // assert
+      expect(result).toEqual(true);
+    });
+
+    it('should return false when value is undefined/null', () => {
+      // arrange
+      const propertyName = 'test';
+      let environment: any = { };
+      process.env = environment;
+      // act
+      const result = helpers.getBooleanPropertyValue(propertyName);
+      // assert
+      expect(result).toEqual(false);
+    });
+
+    it('should return false when value is not "true"', () => {
+      // arrange
+      const propertyName = 'test';
+      const propertyValue = 'taco';
+      let environment: any = { };
+      environment[propertyName] = propertyValue;
+      process.env = environment;
+      // act
+      const result = helpers.getBooleanPropertyValue(propertyName);
+      // assert
+      expect(result).toEqual(false);
+    });
+  });
+});

src/util/helpers.ts

@@ -1,7 +1,7 @@
 import { randomBytes } from 'crypto';
 import { basename, dirname, extname, join } from 'path';
 import { BuildContext, File, HydratedDeepLinkConfigEntry } from './interfaces';
-import { createReadStream, createWriteStream, readFile, readFileSync, readJsonSync, remove, unlink, writeFile } from 'fs-extra';
+import { createReadStream, createWriteStream, readdir, readFile, readFileSync, readJsonSync, remove, unlink, writeFile } from 'fs-extra';
 import * as osName from 'os-name';
 
 let _context: BuildContext;
@@ -155,6 +155,17 @@ export function copyFileAsync(srcPath: string, destPath: string): Promise<any> {
   });
 }
 
+export function readDirAsync(pathToDir: string): Promise<string[]> {
+  return new Promise((resolve, reject) => {
+    readdir(pathToDir, (err: Error, fileNames: string[]) => {
+      if (err) {
+        return reject(err);
+      }
+      resolve(fileNames);
+    });
+  });
+}
+
 export function createFileObject(filePath: string): File {
   const content = readFileSync(filePath).toString();
   return {

src/util/source-maps.spec.ts

@@ -0,0 +1,40 @@
+import { join } from 'path';
+import * as Constants from './constants';
+import * as sourceMaps from './source-maps';
+import * as helpers from './helpers';
+
+describe('source maps', () => {
+  describe('purgeSourceMapsIfNeeded', () => {
+    it('should return a resolved promise when purging source maps isnt needed', () => {
+      // arrange
+      let env: any = {};
+      env[Constants.ENV_VAR_GENERATE_SOURCE_MAP] = 'true';
+      process.env = env;
+      // act
+      const resultPromise = sourceMaps.purgeSourceMapsIfNeeded(null);
+      // assert
+      return resultPromise;
+    });
+
+    it('should return a promise call unlink on all files with a .map extensin', () => {
+      // arrange
+      let env: any = {};
+      env[Constants.ENV_VAR_GENERATE_SOURCE_MAP] = null;
+      process.env = env;
+      const buildDir = '/some/fake/build/dir';
+      const context = { buildDir: buildDir };
+
+      spyOn(helpers, helpers.readDirAsync.name).and.returnValue(Promise.resolve(['test.js', 'test.js.map', 'test2.js', 'test2.js.map']));
+      const unlinkSpy = spyOn(helpers, helpers.unlinkAsync.name).and.returnValue(Promise.resolve());
+
+      // act
+      const resultPromise = sourceMaps.purgeSourceMapsIfNeeded(context);
+
+      // assert
+      return resultPromise.then(() => {
+        expect(unlinkSpy.calls.argsFor(0)[0]).toEqual(join(buildDir, 'test.js.map'));
+        expect(unlinkSpy.calls.argsFor(1)[0]).toEqual(join(buildDir, 'test2.js.map'));
+      });
+    });
+  });
+});

src/util/source-maps.ts

@@ -0,0 +1,20 @@
+import { join } from 'path';
+import * as Constants from './constants';
+import { getBooleanPropertyValue, readDirAsync, unlinkAsync } from './helpers';
+import { BuildContext } from './interfaces';
+
+export function purgeSourceMapsIfNeeded(context: BuildContext) {
+  if (getBooleanPropertyValue(Constants.ENV_VAR_GENERATE_SOURCE_MAP)) {
+    // keep the source maps and just return
+    return Promise.resolve();
+  }
+  return readDirAsync(context.buildDir).then((fileNames: string[]) => {
+    const sourceMaps = fileNames.filter(fileName => fileName.endsWith('.map'));
+    const fullPaths = sourceMaps.map(sourceMap => join(context.buildDir, sourceMap));
+    const promises: Promise<any>[] = [];
+    for (const fullPath of fullPaths) {
+      promises.push(unlinkAsync(fullPath));
+    }
+    return Promise.all(promises);
+  });
+}