Do not minify a bundle when calling wrapModule()

Reviewed By: jeanlauliac Differential Revision: D5791079 fbshipit-source-id: 942d2963e17153508b4a7c5bd48e11b7cb3fdcd4
2017-09-11 08:22:25 -07:00 · 2017-09-11 08:22:25 -07:00 · dcf30322a5
parent 2593ea2ad4
commit dcf30322a5
3 changed files with 112 additions and 149 deletions
--- a/packages/metro-bundler/src/Bundler/Bundle.js
+++ b/packages/metro-bundler/src/Bundler/Bundle.js
@ -86,16 +86,23 @@ class Bundle extends BundleBase {
     * using an instance typed as the base class would be broken. This must be
     * refactored.
     */
-    resolver: {wrapModule: (options: any) => Promise<{code: any, map: any}>},
+    resolver: {
+      wrapModule: (options: any) => {code: any, map: any},
+      minifyModule: ({code: any, map: any, path: any}) => Promise<{
+        code: any,
+        map: any,
+      }>,
+    },
    resolutionResponse: any,
-    module: mixed,
+    module: any,
    /* $FlowFixMe: erroneous change of signature. */
    moduleTransport: ModuleTransport,
    /* $FlowFixMe: erroneous change of signature. */
  ): Promise<void> {
    const index = super.addModule(moduleTransport);
-    return resolver
-      .wrapModule({
+
+    return Promise.resolve(
+      resolver.wrapModule({
        module,
        getModuleId: resolutionResponse.getModuleId,
        dependencyPairs: resolutionResponse.getResolvedDependencyPairs(module),
@ -105,8 +112,13 @@ class Bundle extends BundleBase {
        dependencyOffsets: moduleTransport.meta
          ? moduleTransport.meta.dependencyOffsets
          : undefined,
-        minify: this._minify,
        dev: this._dev,
+      }),
+    )
+      .then(({code, map}) => {
+        return this._minify
+          ? resolver.minifyModule({code, map, path: module.path})
+          : {code, map};
      })
      .then(({code, map}) => {
        // If we get a map from the transformer we'll switch to a mode
--- a/packages/metro-bundler/src/Resolver/tests/Resolver-test.js
+++ b/packages/metro-bundler/src/Resolver/tests/Resolver-test.js
@ -244,38 +244,34 @@ describe('Resolver', function() {
          ]),
      );

-      return depResolver
-        .wrapModule({
-          module: module,
-          getModuleId: resolutionResponse.getModuleId,
-          dependencyPairs: resolutionResponse.getResolvedDependencyPairs(
-            module,
-          ),
-          name: 'test module',
-          code,
-          dependencyOffsets,
-          dev: false,
-        })
-        .then(({code: processedCode}) => {
-          expect(processedCode).toEqual(
-            [
-              '__d(/* test module */function(global, require, module, exports) {' +
-                // require
-                `require(${moduleIds.get('x')}) // ${moduleIds
-                  .get('x')
-                  .trim()} = x`,
-              `require(${moduleIds.get('y')});require(${moduleIds.get(
-                'abc',
-              )}); // ${moduleIds.get('abc').trim()} = abc // ${moduleIds
-                .get('y')
-                .trim()} = y`,
-              "require( 'z' )",
-              'require( "a")',
-              'require("b" )',
-              `}, ${resolutionResponse.getModuleId(module)});`,
-            ].join('\n'),
-          );
-        });
+      const {code: processedCode} = depResolver.wrapModule({
+        module: module,
+        getModuleId: resolutionResponse.getModuleId,
+        dependencyPairs: resolutionResponse.getResolvedDependencyPairs(module),
+        name: 'test module',
+        code,
+        dependencyOffsets,
+        dev: false,
+      });
+
+      expect(processedCode).toEqual(
+        [
+          '__d(/* test module */function(global, require, module, exports) {' +
+            // require
+            `require(${moduleIds.get('x')}) // ${moduleIds
+              .get('x')
+              .trim()} = x`,
+          `require(${moduleIds.get('y')});require(${moduleIds.get(
+            'abc',
+          )}); // ${moduleIds.get('abc').trim()} = abc // ${moduleIds
+            .get('y')
+            .trim()} = y`,
+          "require( 'z' )",
+          'require( "a")',
+          'require("b" )',
+          `}, ${resolutionResponse.getModuleId(module)});`,
+        ].join('\n'),
+      );
    });

    it('should add module transport names as fourth argument to `__d`', () => {
@ -287,28 +283,22 @@ describe('Resolver', function() {
        dependencies: [module],
        mainModuleId: 'test module',
      });
-      return depResolver
-        .wrapModule({
-          getModuleId: resolutionResponse.getModuleId,
-          dependencyPairs: resolutionResponse.getResolvedDependencyPairs(
-            module,
-          ),
-          code,
-          module,
-          name: 'test module',
-          dev: true,
-        })
-        .then(({code: processedCode}) =>
-          expect(processedCode).toEqual(
-            [
-              '__d(/* test module */function(global, require, module, exports) {' +
-                code,
-              `}, ${resolutionResponse.getModuleId(
-                module,
-              )}, null, "test module");`,
-            ].join('\n'),
-          ),
-        );
+
+      const {code: processedCode} = depResolver.wrapModule({
+        getModuleId: resolutionResponse.getModuleId,
+        dependencyPairs: resolutionResponse.getResolvedDependencyPairs(module),
+        code,
+        module,
+        name: 'test module',
+        dev: true,
+      });
+      expect(processedCode).toEqual(
+        [
+          '__d(/* test module */function(global, require, module, exports) {' +
+            code,
+          `}, ${resolutionResponse.getModuleId(module)}, null, "test module");`,
+        ].join('\n'),
+      );
    });

    it('should pass through passed-in source maps', () => {
@ -319,42 +309,39 @@ describe('Resolver', function() {
        mainModuleId: 'test module',
      });
      const inputMap = {version: 3, mappings: 'ARBITRARY'};
-      return depResolver
-        .wrapModule({
-          getModuleId: resolutionResponse.getModuleId,
-          dependencyPairs: resolutionResponse.getResolvedDependencyPairs(
-            module,
-          ),
-          module,
-          name: 'test module',
-          code: 'arbitrary(code)',
-          map: inputMap,
-        })
-        .then(({map}) => expect(map).toBe(inputMap));
+
+      const {map} = depResolver.wrapModule({
+        getModuleId: resolutionResponse.getModuleId,
+        dependencyPairs: resolutionResponse.getResolvedDependencyPairs(module),
+        module,
+        name: 'test module',
+        code: 'arbitrary(code)',
+        map: inputMap,
+      });
+      expect(map).toBe(inputMap);
    });

-    it('should resolve polyfills', function() {
+    it('should resolve polyfills', async function() {
      expect.assertions(1);
      return Resolver.load({
        projectRoot: '/root',
      }).then(depResolver => {
        const polyfill = createPolyfill('test polyfill', []);
        const code = ['global.fetch = () => 1;'].join('');
-        return depResolver
-          .wrapModule({
-            module: polyfill,
-            code,
-          })
-          .then(({code: processedCode}) => {
-            expect(processedCode).toEqual(
-              [
-                '(function(global) {',
-                'global.fetch = () => 1;',
-                '\n})' +
-                  "(typeof global !== 'undefined' ? global : typeof self !== 'undefined' ? self : this);",
-              ].join(''),
-            );
-          });
+
+        const {code: processedCode} = depResolver.wrapModule({
+          module: polyfill,
+          code,
+        });
+
+        expect(processedCode).toEqual(
+          [
+            '(function(global) {',
+            'global.fetch = () => 1;',
+            '\n})' +
+              "(typeof global !== 'undefined' ? global : typeof self !== 'undefined' ? self : this);",
+          ].join(''),
+        );
      });
    });

@ -376,27 +363,25 @@ describe('Resolver', function() {

      it('should prefix JSON files with `module.exports=`', () => {
        expect.assertions(1);
-        return depResolver
-          .wrapModule({
-            getModuleId: resolutionResponse.getModuleId,
-            dependencyPairs: resolutionResponse.getResolvedDependencyPairs(
-              module,
-            ),
+        const {code: processedCode} = depResolver.wrapModule({
+          getModuleId: resolutionResponse.getModuleId,
+          dependencyPairs: resolutionResponse.getResolvedDependencyPairs(
            module,
-            name: id,
-            code,
-            dev: false,
-          })
-          .then(({code: processedCode}) =>
-            expect(processedCode).toEqual(
-              [
-                `__d(/* ${id} */function(global, require, module, exports) {`,
-                `module.exports = ${code}\n}, ${resolutionResponse.getModuleId(
-                  module,
-                )});`,
-              ].join(''),
-            ),
-          );
+          ),
+          module,
+          name: id,
+          code,
+          dev: false,
+        });
+
+        expect(processedCode).toEqual(
+          [
+            `__d(/* ${id} */function(global, require, module, exports) {`,
+            `module.exports = ${code}\n}, ${resolutionResponse.getModuleId(
+              module,
+            )});`,
+          ].join(''),
+        );
      });
    });

@ -419,38 +404,12 @@ describe('Resolver', function() {
        return Resolver.load({
          projectRoot: '/root',
          minifyCode,
+          postMinifyProcess: e => e,
        }).then(r => {
          depResolver = r;
        });
      });

-      it('should invoke the minifier with the wrapped code', () => {
-        expect.assertions(1);
-        const wrappedCode = `__d(/* ${id} */function(global, require, module, exports) {${code}\n}, ${resolutionResponse.getModuleId(
-          module,
-        )});`;
-        return depResolver
-          .wrapModule({
-            getModuleId: resolutionResponse.getModuleId,
-            dependencyPairs: resolutionResponse.getResolvedDependencyPairs(
-              module,
-            ),
-            module,
-            name: id,
-            code,
-            map: sourceMap,
-            minify: true,
-            dev: false,
-          })
-          .then(() => {
-            expect(minifyCode).toBeCalledWith(
-              module.path,
-              wrappedCode,
-              sourceMap,
-            );
-          });
-      });
-
      it('should use minified code', () => {
        expect.assertions(2);
        const minifiedCode = 'minified(code)';
@ -459,15 +418,10 @@ describe('Resolver', function() {
          Promise.resolve({code: minifiedCode, map: minifiedMap}),
        );
        return depResolver
-          .wrapModule({
-            getModuleId: resolutionResponse.getModuleId,
-            dependencyPairs: resolutionResponse.getResolvedDependencyPairs(
-              module,
-            ),
-            module,
+          .minifyModule({
+            path: module.path,
            name: id,
            code,
-            minify: true,
          })
          .then(({code, map}) => {
            expect(code).toEqual(minifiedCode);
--- a/packages/metro-bundler/src/Resolver/index.js
+++ b/packages/metro-bundler/src/Resolver/index.js
@ -227,7 +227,6 @@ class Resolver {
    map,
    code,
    dev = true,
-    minify = false,
  }: {
    module: Module,
    getModuleId: ({path: string}) => number,
@ -237,8 +236,7 @@ class Resolver {
    map: ?MappingsMap,
    code: string,
    dev?: boolean,
-    minify?: boolean,
-  }): Promise<{code: string, map: ?MappingsMap}> {
+  }): {code: string, map: ?MappingsMap} {
    if (module.isJSON()) {
      code = `module.exports = ${code}`;
    }
@ -258,12 +256,10 @@ class Resolver {
      code = defineModuleCode(moduleId, code, name, dev);
    }

-    return minify
-      ? this._minifyCode(module.path, code, map).then(this._postMinifyProcess)
-      : Promise.resolve({code, map});
+    return {code, map};
  }

-  minifyModule({
+  async minifyModule({
    path,
    code,
    map,
@ -272,7 +268,8 @@ class Resolver {
    code: string,
    map: ?MappingsMap,
  }): Promise<{code: string, map: ?MappingsMap}> {
-    return this._minifyCode(path, code, map);
+    const minified = await this._minifyCode(path, code, map);
+    return await this._postMinifyProcess(minified);
  }

  getDependencyGraph(): DependencyGraph {