metro-bundler: ModuleGraph tests: @flow

Summary: Having types in tests ensure that we're testing what the API is supposed to process and not something else. In that case, adding type revealed that we were mistakenly passing strings instead of `Buffer` objects to the transform and optimize functions, but it would still work because `toString` was called over it. Passing proper `Buffer` objects as expected ensures that it doesn't just work with strings, and that the integration of these files in the rest of the application works as expected. This changeset fixes these callsites and add a few invariants here and there. We use `invariant` instead of `expect()` because Flow understands only the former as a type refinement, even though `expect` would also be a form of possible refinement. I don't think it's a big deal.

Reviewed By: davidaurelio

Differential Revision: D6160019

fbshipit-source-id: cbcbb05d7bccf9e1b9f6bb3ea30b0bb2925aef1b

packages/metro-bundler/src/ModuleGraph/types.flow.js

@@ -150,7 +150,7 @@ export type TransformResult = {|
 
 export type TransformResults = {[string]: TransformResult};
 
-export type TransformVariants = {+[name: string]: {}, +default: {}};
+export type TransformVariants = {+[name: string]: {}};
 
 export type TransformedCodeFile = {|
   +file: string,

packages/metro-bundler/src/ModuleGraph/worker/__tests__/collect-dependencies-test.js

@@ -6,8 +6,9 @@
  * LICENSE file in the root directory of this source tree. An additional grant
  * of patent rights can be found in the PATENTS file in the same directory.
  *
- * @format
  * @emails oncall+javascript_foundation
+ * @flow
+ * @format
  */
 
 'use strict';

packages/metro-bundler/src/ModuleGraph/worker/__tests__/optimize-module-test.js

@@ -6,17 +6,21 @@
  * LICENSE file in the root directory of this source tree. An additional grant
  * of patent rights can be found in the PATENTS file in the same directory.
  *
- * @format
  * @emails oncall+javascript_foundation
+ * @flow
+ * @format
  */
 
 'use strict';
 
+const invariant = require('fbjs/lib/invariant');
+const nullthrows = require('fbjs/lib/nullthrows');
 const optimizeModule = require('../optimize-module');
 const transformModule = require('../transform-module');
 const transformer = require('../../../transformer.js');
-const {SourceMapConsumer} = require('source-map');
+
 const {fn} = require('../../test-helpers');
+const {SourceMapConsumer} = require('source-map');
 
 const {objectContaining} = jasmine;
 
@@ -27,21 +31,27 @@ describe('optimizing JS modules', () => {
     platform: 'android',
     postMinifyProcess: x => x,
   };
-  const originalCode = `if (Platform.OS !== 'android') {
+  const originalCode = new Buffer(
+    `if (Platform.OS !== 'android') {
       require('arbitrary-dev');
     } else {
       __DEV__ ? require('arbitrary-android-dev') : require('arbitrary-android-prod');
-    }`;
+    }`,
+    'utf8',
+  );
 
   let transformResult;
   beforeAll(() => {
     const result = transformModule(originalCode, {filename, transformer});
-    transformResult = JSON.stringify({type: 'code', details: result.details});
+    transformResult = new Buffer(
+      JSON.stringify({type: 'code', details: result.details}),
+      'utf8',
+    );
   });
 
   it('copies everything from the transformed file, except for transform results', () => {
     const result = optimizeModule(transformResult, optimizationOptions);
-    const expected = JSON.parse(transformResult).details;
+    const expected = JSON.parse(transformResult.toString('utf8')).details;
     delete expected.transformed;
     expect(result.type).toBe('code');
     expect(result.details).toEqual(objectContaining(expected));
@@ -51,14 +61,19 @@ describe('optimizing JS modules', () => {
     let dependencyMapName, injectedVars, optimized, requireName;
     beforeAll(() => {
       const result = optimizeModule(transformResult, optimizationOptions);
+      invariant(result.type === 'code', 'result must be code');
       optimized = result.details.transformed.default;
-      injectedVars = optimized.code.match(/function\(([^)]*)/)[1].split(',');
+      injectedVars = nullthrows(
+        optimized.code.match(/function\(([^)]*)/),
+      )[1].split(',');
       [, requireName, , , dependencyMapName] = injectedVars;
     });
 
     it('optimizes code', () => {
       expect(optimized.code).toEqual(
-        `__d(function(${injectedVars}){${requireName}(${dependencyMapName}[0])});`,
+        `__d(function(${injectedVars.join(
+          ',',
+        )}){${requireName}(${dependencyMapName}[0])});`,
       );
     });
 
@@ -69,7 +84,9 @@ describe('optimizing JS modules', () => {
     it('creates source maps', () => {
       const consumer = new SourceMapConsumer(optimized.map);
       const column = optimized.code.lastIndexOf(requireName + '(');
-      const loc = findLast(originalCode, 'require');
+      const loc = nullthrows(
+        findLast(originalCode.toString('utf8'), 'require'),
+      );
 
       expect(consumer.originalPositionFor({line: 1, column})).toEqual(
         objectContaining(loc),
@@ -80,8 +97,9 @@ describe('optimizing JS modules', () => {
       const result = optimizeModule(transformResult, {
         ...optimizationOptions,
         isPolyfill: true,
-      }).details;
+      });
-      expect(result.transformed.default.dependencies).toEqual([]);
+      invariant(result.type === 'code', 'result must be code');
+      expect(result.details.transformed.default.dependencies).toEqual([]);
     });
   });
 
@@ -99,6 +117,7 @@ describe('optimizing JS modules', () => {
     it('passes the result to the provided postprocessing function', () => {
       postMinifyProcess.stub.callsFake(x => x);
       const result = optimize();
+      invariant(result.type === 'code', 'result must be code');
       const {code, map} = result.details.transformed.default;
       expect(postMinifyProcess).toBeCalledWith({code, map});
     });
@@ -107,7 +126,9 @@ describe('optimizing JS modules', () => {
       const code = 'var postprocessed = "code";';
       const map = {version: 3, mappings: 'postprocessed'};
       postMinifyProcess.stub.returns({code, map});
-      expect(optimize().details.transformed.default).toEqual(
+      const result = optimize();
+      invariant(result.type === 'code', 'result must be code');
+      expect(result.details.transformed.default).toEqual(
         objectContaining({code, map}),
       );
     });
@@ -116,7 +137,11 @@ describe('optimizing JS modules', () => {
   it('passes through non-code data unmodified', () => {
     const data = {type: 'asset', details: {arbitrary: 'data'}};
     expect(
-      optimizeModule(JSON.stringify(data), {dev: true, platform: ''}),
+      optimizeModule(new Buffer(JSON.stringify(data), 'utf8'), {
+        dev: true,
+        platform: '',
+        postMinifyProcess: ({code, map}) => ({code, map}),
+      }),
     ).toEqual(data);
   });
 });

packages/metro-bundler/src/ModuleGraph/worker/__tests__/transform-module-test.js

@@ -6,21 +6,26 @@
  * LICENSE file in the root directory of this source tree. An additional grant
  * of patent rights can be found in the PATENTS file in the same directory.
  *
- * @format
  * @emails oncall+javascript_foundation
+ * @flow
+ * @format
  */
 
 'use strict';
 
+const invariant = require('fbjs/lib/invariant');
+const nullthrows = require('fbjs/lib/nullthrows');
+const t = require('babel-types');
 const transformModule = require('../transform-module');
 
-const t = require('babel-types');
-const {SourceMapConsumer} = require('source-map');
 const {fn} = require('../../test-helpers');
 const {parse} = require('babylon');
+const {SourceMapConsumer} = require('source-map');
 const generate = require('babel-generator').default;
 const {traverse} = require('babel-core');
 
+import type {TransformVariants} from '../../types.flow';
+
 jest.mock('image-size', () => buffer => {
   return JSON.parse(buffer.toString('utf8')).__size;
 });
@@ -33,13 +38,14 @@ describe('transforming JS modules:', () => {
   beforeEach(() => {
     transformer = {
       transform: fn(),
+      getCacheKey: () => 'foo',
     };
     transformer.transform.stub.returns(transformResult());
   });
 
   const {bodyAst, sourceCode, transformedCode} = createTestData();
 
-  const options = variants => ({
+  const options = (variants?: TransformVariants) => ({
     filename,
     transformer,
     variants,
@@ -61,7 +67,7 @@ describe('transforming JS modules:', () => {
 
   it('exposes a haste ID if present', () => {
     const hasteID = 'TheModule';
-    const codeWithHasteID = `/** @providesModule ${hasteID} */`;
+    const codeWithHasteID = toBuffer(`/** @providesModule ${hasteID} */`);
     const result = transformModule(codeWithHasteID, options());
     expect(result.type).toBe('code');
     expect(result.details).toEqual(expect.objectContaining({hasteID}));
@@ -88,27 +94,23 @@ describe('transforming JS modules:', () => {
     projectRoot: '',
   };
 
-  it(
+  it('calls the passed-in transform function with code, file name, and options for all passed in variants', () => {
-    'calls the passed-in transform function with code, file name, and options ' +
+    const variants = {dev: {dev: true}, prod: {dev: false}};
-      'for all passed in variants',
-    () => {
-      const variants = {dev: {dev: true}, prod: {dev: false}};
 
-      transformModule(sourceCode, options(variants));
+    transformModule(sourceCode, options(variants));
-      expect(transformer.transform).toBeCalledWith({
+    expect(transformer.transform).toBeCalledWith({
-        filename,
+      filename,
-        localPath: filename,
+      localPath: filename,
-        options: {...defaults, ...variants.dev},
+      options: {...defaults, ...variants.dev},
-        src: sourceCode,
+      src: sourceCode.toString('utf8'),
-      });
+    });
-      expect(transformer.transform).toBeCalledWith({
+    expect(transformer.transform).toBeCalledWith({
-        filename,
+      filename,
-        localPath: filename,
+      localPath: filename,
-        options: {...defaults, ...variants.prod},
+      options: {...defaults, ...variants.prod},
-        src: sourceCode,
+      src: sourceCode.toString('utf8'),
-      });
+    });
-    },
+  });
-  );
 
   it('calls back with any error yielded by the transform function', () => {
     const error = new Error();
@@ -124,7 +126,9 @@ describe('transforming JS modules:', () => {
   it('wraps the code produced by the transform function into a module factory', () => {
     const result = transformModule(sourceCode, options());
 
+    invariant(result.type === 'code', 'result must be code');
     const {code, dependencyMapName} = result.details.transformed.default;
+    invariant(dependencyMapName != null, 'dependencyMapName cannot be null');
     expect(code.replace(/\s+/g, '')).toEqual(
       `__d(function(global,require,module,exports,${dependencyMapName}){${transformedCode}});`,
     );
@@ -132,6 +136,7 @@ describe('transforming JS modules:', () => {
 
   it('wraps the code produced by the transform function into an IIFE for polyfills', () => {
     const result = transformModule(sourceCode, {...options(), polyfill: true});
+    invariant(result.type === 'code', 'result must be code');
     const {code} = result.details.transformed.default;
     expect(code.replace(/\s+/g, '')).toEqual(
       `(function(global){${transformedCode}})(this);`,
@@ -140,6 +145,7 @@ describe('transforming JS modules:', () => {
 
   it('creates source maps', () => {
     const result = transformModule(sourceCode, options());
+    invariant(result.type === 'code', 'result must be code');
     const {code, map} = result.details.transformed.default;
     const position = findColumnAndLine(code, 'code');
     expect(position).not.toBeNull();
@@ -157,7 +163,8 @@ describe('transforming JS modules:', () => {
     const {body} = parse(code).program;
     transformer.transform.stub.returns(transformResult(body));
 
-    const result = transformModule(code, options());
+    const result = transformModule(toBuffer(code), options());
+    invariant(result.type === 'code', 'result must be code');
     expect(result.details.transformed.default).toEqual(
       expect.objectContaining({dependencies: [dep1, dep2]}),
     );
@@ -172,19 +179,28 @@ describe('transforming JS modules:', () => {
       .returns(transformResult([]));
 
     const result = transformModule(sourceCode, options(variants));
+    invariant(result.type === 'code', 'result must be code');
     const {dev, prod} = result.details.transformed;
     expect(dev.code.replace(/\s+/g, '')).toEqual(
-      `__d(function(global,require,module,exports,${dev.dependencyMapName}){arbitrary(code);});`,
+      `__d(function(global,require,module,exports,${nullthrows(
+        dev.dependencyMapName,
+      )}){arbitrary(code);});`,
     );
     expect(prod.code.replace(/\s+/g, '')).toEqual(
-      `__d(function(global,require,module,exports,${prod.dependencyMapName}){arbitrary(code);});`,
+      `__d(function(global,require,module,exports,${nullthrows(
+        prod.dependencyMapName,
+      )}){arbitrary(code);});`,
     );
   });
 
   it('prefixes JSON files with `module.exports = `', () => {
     const json = '{"foo":"bar"}';
 
-    const result = transformModule(json, {...options(), filename: 'some.json'});
+    const result = transformModule(toBuffer(json), {
+      ...options(),
+      filename: 'some.json',
+    });
+    invariant(result.type === 'code', 'result must be code');
     const {code} = result.details.transformed.default;
     expect(code.replace(/\s+/g, '')).toEqual(
       '__d(function(global,require,module,exports){' +
@@ -193,7 +209,11 @@ describe('transforming JS modules:', () => {
   });
 
   it('does not create source maps for JSON files', () => {
-    const result = transformModule('{}', {...options(), filename: 'some.json'});
+    const result = transformModule(toBuffer('{}'), {
+      ...options(),
+      filename: 'some.json',
+    });
+    invariant(result.type === 'code', 'result must be code');
     expect(result.details.transformed.default).toEqual(
       expect.objectContaining({map: null}),
     );
@@ -207,10 +227,11 @@ describe('transforming JS modules:', () => {
       'react-native': {'react-native': 'defs'},
     };
 
-    const result = transformModule(JSON.stringify(pkg), {
+    const result = transformModule(toBuffer(JSON.stringify(pkg)), {
       ...options(),
       filename: 'arbitrary/package.json',
     });
+    invariant(result.type === 'code', 'result must be code');
     expect(result.details.package).toEqual(pkg);
   });
 
@@ -219,7 +240,7 @@ describe('transforming JS modules:', () => {
       const image = {__size: {width: 30, height: 20}};
       ['foo.png', 'foo@2x.ios.png'].forEach(filePath => {
         expect(
-          transformModule(new Buffer(JSON.stringify(image), 'utf8'), {
+          transformModule(toBuffer(JSON.stringify(image)), {
             ...options(),
             filename: filePath,
           }),
@@ -249,7 +270,7 @@ function createTestData() {
   });
   return {
     bodyAst: fileAst.program.body,
-    sourceCode,
+    sourceCode: toBuffer(sourceCode),
     transformedCode: generate(fileAst).code,
   };
 }
@@ -265,3 +286,7 @@ function findColumnAndLine(text, string) {
   }
   return null;
 }
+
+function toBuffer(str) {
+  return new Buffer(str, 'utf8');
+}

packages/metro-bundler/src/ModuleGraph/worker/__tests__/wrap-worker-fn-test.js

@@ -6,8 +6,9 @@
  * LICENSE file in the root directory of this source tree. An additional grant
  * of patent rights can be found in the PATENTS file in the same directory.
  *
- * @format
  * @emails oncall+javascript_foundation
+ * @flow
+ * @format
  */
 
 'use strict';