Add minify support to the Delta Bundler

Reviewed By: davidaurelio

Differential Revision: D6163566

fbshipit-source-id: d49eab159ee1d1b4453136f21e25e48154d0142a

packages/metro-bundler/src/Bundler/Bundle.js

@@ -133,7 +133,14 @@ class Bundle extends BundleBase {
         // If we get a map from the transformer we'll switch to a mode
         // were we're combining the source maps as opposed to
         if (map) {
-          const usesRawMappings = isRawMappings(map);
+          let usesRawMappings = isRawMappings(map);
+
+          // Transform the raw mappings into standard source maps so the RAM
+          // bundler for production can build the source maps correctly.
+          if (usesRawMappings) {
+            map = fromRawMappings(module).toMap(undefined, {});
+            usesRawMappings = false;
+          }
 
           if (this._sourceMapFormat === 'undetermined') {
             this._sourceMapFormat = usesRawMappings ? 'flattened' : 'indexed';

packages/metro-bundler/src/Bundler/source-map/__tests__/__snapshots__/source-map-test.js.snap

@@ -0,0 +1,126 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[` 1`] = `
+Array [
+  Object {
+    "generated": Object {
+      "column": 2,
+      "line": 1,
+    },
+    "name": null,
+    "original": Object {
+      "column": null,
+      "line": null,
+    },
+    "source": null,
+  },
+  Object {
+    "generated": Object {
+      "column": 4,
+      "line": 3,
+    },
+    "name": "apples",
+    "original": Object {
+      "column": 6,
+      "line": 5,
+    },
+    "source": "path1",
+  },
+  Object {
+    "generated": Object {
+      "column": 8,
+      "line": 7,
+    },
+    "name": null,
+    "original": Object {
+      "column": 10,
+      "line": 9,
+    },
+    "source": "path1",
+  },
+  Object {
+    "generated": Object {
+      "column": 12,
+      "line": 11,
+    },
+    "name": "pears",
+    "original": Object {
+      "column": 14,
+      "line": 13,
+    },
+    "source": "path1",
+  },
+  Object {
+    "generated": Object {
+      "column": 2,
+      "line": 12,
+    },
+    "name": null,
+    "original": Object {
+      "column": null,
+      "line": null,
+    },
+    "source": null,
+  },
+  Object {
+    "generated": Object {
+      "column": 4,
+      "line": 14,
+    },
+    "name": "bananas",
+    "original": Object {
+      "column": 16,
+      "line": 15,
+    },
+    "source": "path2",
+  },
+  Object {
+    "generated": Object {
+      "column": 12,
+      "line": 25,
+    },
+    "name": null,
+    "original": Object {
+      "column": null,
+      "line": null,
+    },
+    "source": null,
+  },
+  Object {
+    "generated": Object {
+      "column": 14,
+      "line": 27,
+    },
+    "name": "bananas",
+    "original": Object {
+      "column": 16,
+      "line": 15,
+    },
+    "source": "path3",
+  },
+  Object {
+    "generated": Object {
+      "column": 18,
+      "line": 31,
+    },
+    "name": null,
+    "original": Object {
+      "column": 110,
+      "line": 19,
+    },
+    "source": "path3",
+  },
+  Object {
+    "generated": Object {
+      "column": 112,
+      "line": 35,
+    },
+    "name": "pears",
+    "original": Object {
+      "column": 114,
+      "line": 113,
+    },
+    "source": "path3",
+  },
+]
+`;

packages/metro-bundler/src/Bundler/source-map/__tests__/source-map-test.js

@@ -13,7 +13,8 @@
 'use strict';
 
 const Generator = require('../Generator');
-const {compactMapping, fromRawMappings} = require('..');
+
+const {compactMapping, fromRawMappings, toRawMappings} = require('..');
 
 describe('flattening mappings / compacting', () => {
   it('flattens simple mappings', () => {
@@ -89,6 +90,19 @@ describe('build map from raw mappings', () => {
       version: 3,
     });
   });
+
+  describe('convert a sourcemap into raw mappings', () => {
+    expect(
+      toRawMappings({
+        mappings:
+          'E;;IAIMA;;;;QAII;;;;YAIIC;E;;ICEEC;;;;;;;;;;;Y;;cCAAA;;;;kBAI8F;;;;gHA8FID',
+        names: ['apples', 'pears', 'bananas'],
+        sources: ['path1', 'path2', 'path3'],
+        sourcesContent: ['code1', 'code2', 'code3'],
+        version: 3,
+      }),
+    ).toMatchSnapshot();
+  });
 });
 
 const lines = n => Array(n).join('\n');

packages/metro-bundler/src/Bundler/source-map/source-map.js

@@ -13,8 +13,10 @@
 'use strict';
 
 const Generator = require('./Generator');
+const SourceMap = require('source-map');
 
 import type ModuleTransport from '../../lib/ModuleTransport';
+import type {MappingsMap, RawMappings} from '../../lib/SourceMap';
 import type {RawMapping as BabelRawMapping} from 'babel-generator';
 
 type GeneratedCodeMapping = [number, number];
@@ -60,6 +62,31 @@ function fromRawMappings(
   return generator;
 }
 
+/**
+ * Transforms a standard source map object into a Raw Mappings object, to be
+ * used across the bundler.
+ */
+function toRawMappings(sourceMap: MappingsMap): RawMappings {
+  const rawMappings = [];
+
+  new SourceMap.SourceMapConsumer(sourceMap).eachMapping(map => {
+    rawMappings.push({
+      generated: {
+        line: map.generatedLine,
+        column: map.generatedColumn,
+      },
+      original: {
+        line: map.originalLine,
+        column: map.originalColumn,
+      },
+      source: map.source,
+      name: map.name,
+    });
+  });
+
+  return rawMappings;
+}
+
 function compactMapping(mapping: BabelRawMapping): RawMapping {
   const {column, line} = mapping.generated;
   const {name, original} = mapping;
@@ -116,5 +143,8 @@ function countLines(string) {
   return string.split('\n').length;
 }
 
-exports.fromRawMappings = fromRawMappings;
-exports.compactMapping = compactMapping;
+module.exports = {
+  fromRawMappings,
+  toRawMappings,
+  compactMapping,
+};

packages/metro-bundler/src/DeltaBundler/DeltaTransformer.js

@@ -15,14 +15,14 @@
 const DeltaCalculator = require('./DeltaCalculator');
 
 const createModuleIdFactory = require('../lib/createModuleIdFactory');
+const minify = require('../JSTransformer/worker/minify');
 
 const {EventEmitter} = require('events');
 
-import type {RawMapping} from '../Bundler/source-map';
 import type Bundler from '../Bundler';
 import type {Options as JSTransformerOptions} from '../JSTransformer/worker';
 import type Resolver from '../Resolver';
-import type {MappingsMap} from '../lib/SourceMap';
+import type {CompactRawMappings} from '../lib/SourceMap';
 import type Module from '../node-haste/Module';
 import type {Options as BundleOptions} from './';
 import type {DependencyEdges} from './traverseDependencies';
@@ -37,7 +37,7 @@ export type DeltaEntryType =
 export type DeltaEntry = {|
   +code: string,
   +id: number,
-  +map: ?Array<RawMapping>,
+  +map: ?CompactRawMappings,
   +name: string,
   +path: string,
   +source: string,
@@ -379,18 +379,16 @@ class DeltaTransformer extends EventEmitter {
           map: metadata.map,
         };
 
-    // Ignore the Source Maps if the output of the transformer is not our
-    // custom rawMapping data structure, since the Delta bundler cannot process
-    // them. This can potentially happen when the minifier is enabled (since
-    // uglifyJS only returns standard Source Maps).
-    const map = Array.isArray(wrapped.map) ? wrapped.map : undefined;
+    const {code, map} = transformOptions.minify
+      ? minify.withRawMappings(wrapped.code, wrapped.map, module.path)
+      : wrapped;
 
     const id = this._getModuleId(module);
 
     return [
       id,
       {
-        code: ';' + wrapped.code,
+        code: ';' + code,
         id,
         map,
         name,
@@ -419,7 +417,7 @@ class DeltaTransformer extends EventEmitter {
   ): Promise<{
     +code: string,
     +dependencyOffsets: ?Array<number>,
-    +map: ?MappingsMap,
+    +map: ?CompactRawMappings,
     +source: string,
   }> {
     if (module.isAsset()) {

packages/metro-bundler/src/DeltaBundler/index.js

@@ -68,10 +68,7 @@ class DeltaBundler {
       deltaTransformer = await DeltaTransformer.create(
         this._bundler,
         this._options,
-        {
-          ...options, // The Delta Bundler does not support minifying due to
-          minify: false, // issues generating the source maps (T21699790).
-        },
+        options,
       );
 
       this._deltaTransformers.set(bundleId, deltaTransformer);

packages/metro-bundler/src/JSTransformer/worker/__tests__/worker-test.js

@@ -32,7 +32,7 @@ describe('code transformation worker:', () => {
     transformer = {
       transform: jest.fn(({filename, options, src}) => ({
         code: src,
-        map: {},
+        map: [],
       })),
     };
   });
@@ -104,7 +104,7 @@ describe('code transformation worker:', () => {
   it('calls back with the result of the transform in the cache', done => {
     const result = {
       code: 'some.other(code)',
-      map: {},
+      map: [],
     };
 
     transformCode(

packages/metro-bundler/src/JSTransformer/worker/index.js

@@ -16,13 +16,16 @@ const asyncify = require('async/asyncify');
 const constantFolding = require('./constant-folding');
 const extractDependencies = require('./extract-dependencies');
 const inline = require('./inline');
-const invariant = require('fbjs/lib/invariant');
 const minify = require('./minify');
 
-const {compactMapping} = require('../../Bundler/source-map');
+const {compactMapping, toRawMappings} = require('../../Bundler/source-map');
 
 import type {LogEntry} from '../../Logger/Types';
-import type {MappingsMap} from '../../lib/SourceMap';
+import type {
+  CompactRawMappings,
+  MappingsMap,
+  RawMappings,
+} from '../../lib/SourceMap';
 import type {LocalPath} from '../../node-haste/lib/toLocalPath';
 import type {Ast, Plugins as BabelPlugins} from 'babel-core';
 
@@ -30,7 +33,7 @@ export type TransformedCode = {
   code: string,
   dependencies: Array<string>,
   dependencyOffsets: Array<number>,
-  map?: ?MappingsMap,
+  map?: ?CompactRawMappings,
 };
 
 export type Transform<ExtraOptions: {}> = ({|
@@ -39,7 +42,7 @@ export type Transform<ExtraOptions: {}> = ({|
   options: ExtraOptions & TransformOptions,
   plugins?: BabelPlugins,
   src: string,
-|}) => {ast: ?Ast, code: string, map: ?MappingsMap};
+|}) => {ast: ?Ast, code: string, map: ?MappingsMap | RawMappings};
 
 export type Transformer<ExtraOptions: {} = {}> = {
   transform: Transform<ExtraOptions>,
@@ -97,11 +100,6 @@ const transformCode: TransformCode = asyncify(
     sourceCode: string,
     options: Options,
   ): Data => {
-    invariant(
-      !options.minify || options.transform.generateSourceMaps,
-      'Minifying source code requires the `generateSourceMaps` option to be `true`',
-    );
-
     const isJson = filename.endsWith('.json');
     if (isJson) {
       sourceCode = 'module.exports=' + sourceCode;
@@ -127,18 +125,17 @@ const transformCode: TransformCode = asyncify(
       src: sourceCode,
     });
 
-    // TODO: Add more robust check once the transformer only returns rawMappings
-    if (Array.isArray(transformed.map)) {
-      transformed.map = transformed.map.map(compactMapping);
-    }
+    // If the transformer returns standard sourcemaps, we need to transform them
+    // to rawMappings so we can process them correctly.
+    const rawMappings =
+      transformed.map && !Array.isArray(transformed.map)
+        ? toRawMappings(transformed.map)
+        : transformed.map;
 
-    invariant(
-      transformed != null,
-      'Missing transform results despite having no error.',
-    );
-
-    let {code, map} = transformed;
+    // Convert the sourcemaps to Compact Raw source maps.
+    const map = rawMappings ? rawMappings.map(compactMapping) : null;
 
+    let code = transformed.code;
     if (isJson) {
       code = code.replace(/^\w+\.exports=/, '');
     } else {
@@ -191,7 +188,7 @@ exports.transformAndExtractDependencies = (
 };
 
 exports.minify = asyncify(
-  (filename: string, code: string, sourceMap: MappingsMap) => {
+  (filename: string, code: string, sourceMap: RawMappings) => {
     var result;
     try {
       result = minify.withSourceMap(code, sourceMap, filename);

packages/metro-bundler/src/JSTransformer/worker/minify.js

@@ -14,10 +14,21 @@
 
 const uglify = require('uglify-es');
 
-import type {MappingsMap} from '../../lib/SourceMap';
+const {
+  compactMapping,
+  fromRawMappings,
+  toRawMappings,
+} = require('../../Bundler/source-map');
+
+import type {
+  CompactRawMappings,
+  MappingsMap,
+  RawMappings,
+} from '../../lib/SourceMap';
+
 type ResultWithMap = {
   code: string,
-  map: MappingsMap,
+  map: ?MappingsMap,
 };
 
 function noSourceMap(code: string): string {
@@ -26,9 +37,15 @@ function noSourceMap(code: string): string {
 
 function withSourceMap(
   code: string,
-  sourceMap: ?MappingsMap,
+  sourceMap: ?MappingsMap | RawMappings,
   filename: string,
 ): ResultWithMap {
+  if (sourceMap && Array.isArray(sourceMap)) {
+    sourceMap = fromRawMappings([
+      {code, source: code, map: sourceMap, path: filename},
+    ]).toMap(undefined, {});
+  }
+
   const result = minify(code, sourceMap);
 
   const map: MappingsMap = JSON.parse(result.map);
@@ -36,6 +53,19 @@ function withSourceMap(
   return {code: result.code, map};
 }
 
+function withRawMappings(
+  code: string,
+  map: ?RawMappings,
+  filename: string,
+): {code: string, map: ?CompactRawMappings} {
+  const result = withSourceMap(code, map, filename);
+
+  return {
+    code: result.code,
+    map: result.map ? toRawMappings(result.map).map(compactMapping) : undefined,
+  };
+}
+
 function minify(inputCode: string, inputMap: ?MappingsMap) {
   const result = uglify.minify(inputCode, {
     mangle: {toplevel: true},
@@ -63,5 +93,6 @@ function minify(inputCode: string, inputMap: ?MappingsMap) {
 
 module.exports = {
   noSourceMap,
+  withRawMappings,
   withSourceMap,
 };

packages/metro-bundler/src/Resolver/index.js

@@ -19,7 +19,7 @@ const pathJoin = require('path').join;
 
 import type ResolutionResponse from '../node-haste/DependencyGraph/ResolutionResponse';
 import type Module, {HasteImpl, TransformCode} from '../node-haste/Module';
-import type {MappingsMap} from '../lib/SourceMap';
+import type {MappingsMap, CompactRawMappings} from '../lib/SourceMap';
 import type {PostMinifyProcess} from '../Bundler';
 import type {Options as JSTransformerOptions} from '../JSTransformer/worker';
 import type {Reporter} from '../lib/reporting';
@@ -230,10 +230,10 @@ class Resolver {
     dependencyPairs: Map<string, string>,
     dependencyOffsets: Array<number>,
     name: string,
-    map: ?MappingsMap,
+    map: ?CompactRawMappings,
     code: string,
     dev?: boolean,
-  }): {code: string, map: ?MappingsMap} {
+  }): {code: string, map: ?CompactRawMappings} {
     if (module.isJSON()) {
       code = `module.exports = ${code}`;
     }

packages/metro-bundler/src/lib/SourceMap.js

@@ -13,12 +13,16 @@
 'use strict';
 
 import type {SourceMap as MappingsMap} from 'babel-core';
+import type {RawMapping} from 'babel-generator';
+import type {RawMapping as CompactRawMapping} from 'source-map';
 
 export type IndexMapSection = {
   map: SourceMap,
   offset: {line: number, column: number},
 };
 
+export type RawMappings = Array<RawMapping>;
+
 type FBExtensions = {x_facebook_offsets: Array<number>};
 
 export type {MappingsMap};
@@ -33,6 +37,8 @@ export type FBIndexMap = IndexMap & FBExtensions;
 export type SourceMap = IndexMap | MappingsMap;
 export type FBSourceMap = FBIndexMap | (MappingsMap & FBExtensions);
 
+export type CompactRawMappings = Array<CompactRawMapping>;
+
 function isMappingsMap(map: SourceMap): %checks {
   return map.mappings !== undefined;
 }

packages/metro-bundler/src/lib/TransformCaching.js

@@ -22,7 +22,7 @@ const rimraf = require('rimraf');
 const writeFileAtomicSync = require('write-file-atomic').sync;
 
 import type {Options as WorkerOptions} from '../JSTransformer/worker';
-import type {MappingsMap} from './SourceMap';
+import type {CompactRawMappings} from './SourceMap';
 import type {Reporter} from './reporting';
 import type {LocalPath} from '../node-haste/lib/toLocalPath';
 
@@ -35,7 +35,7 @@ export type CachedResult = {
   code: string,
   dependencies: Array<string>,
   dependencyOffsets: Array<number>,
-  map?: ?MappingsMap,
+  map?: ?CompactRawMappings,
 };
 
 export type TransformCacheResult = {|
@@ -336,7 +336,7 @@ function readMetadataFileSync(
   cachedSourceHash: string,
   dependencies: Array<string>,
   dependencyOffsets: Array<number>,
-  sourceMap: ?MappingsMap,
+  sourceMap: ?CompactRawMappings,
 } {
   const metadataStr = fs.readFileSync(metadataFilePath, 'utf8');
   const metadata = tryParseJSON(metadataStr);