Always read the module source code from the worker

Reviewed By: jeanlauliac

Differential Revision: D7894133

fbshipit-source-id: bf5c366d31e6e30d0cc82bd32fa9668040aea081

packages/metro/src/Bundler.js

@@ -217,7 +217,6 @@ class Bundler {
 
   async _cachedTransformCode(
     module: Module,
-    code: ?string,
     transformCodeOptions: WorkerOptions,
   ): Promise<TransformedCode> {
     const cache = this._cache;
@@ -229,6 +228,7 @@ class Bundler {
       dev,
       hot,
       inlineRequires,
+      isScript,
       minify,
       platform,
       projectRoot: _projectRoot, // Blacklisted property.
@@ -257,6 +257,7 @@ class Bundler {
       dev,
       hot,
       inlineRequires,
+      isScript,
       minify,
       platform,
     ]);
@@ -272,8 +273,6 @@ class Bundler {
       : await this._transformer.transform(
           module.path,
           module.localPath,
-          code,
-          module.isPolyfill(),
           transformCodeOptions,
           this._opts.assetExts,
           this._opts.assetRegistryPath,

packages/metro/src/Bundler/__tests__/Bundler-test.js

@@ -127,7 +127,7 @@ describe('Bundler', function() {
       result: {},
     });
 
-    await bundlerInstance._cachedTransformCode(module, null, {});
+    await bundlerInstance._cachedTransformCode(module, {});
 
     // We got the SHA-1 of the file from the dependency graph.
     expect(depGraph.getSha1).toBeCalledWith('/root/foo.js');

packages/metro/src/JSTransformer.js

@@ -86,8 +86,6 @@ module.exports = class Transformer {
   async transform(
     filename: string,
     localPath: LocalPath,
-    code: ?string,
-    isScript: boolean,
     options: WorkerOptions,
     assetExts: $ReadOnlyArray<string>,
     assetRegistryPath: string,
@@ -99,9 +97,7 @@ module.exports = class Transformer {
       const data = await this._worker.transform(
         filename,
         localPath,
-        code,
         this._transformModulePath,
-        isScript,
         options,
         assetExts,
         assetRegistryPath,

packages/metro/src/JSTransformer/__tests__/Transformer-test.js

@@ -68,13 +68,10 @@ describe('Transformer', function() {
 
   it('passes transform data to the worker farm when transforming', async () => {
     const transformOptions = {arbitrary: 'options'};
-    const code = 'arbitrary(code)';
 
     await new Transformer(opts).transform(
       fileName,
       localPath,
-      code,
-      false,
       transformOptions,
       [],
       '',
@@ -84,9 +81,7 @@ describe('Transformer', function() {
     expect(api.transform).toBeCalledWith(
       fileName,
       localPath,
-      code,
       transformModulePath,
-      false,
       transformOptions,
       [],
       '',
@@ -102,7 +97,7 @@ describe('Transformer', function() {
     const snippet = 'snippet';
 
     api.transform.mockImplementation(
-      (filename, localPth, code, transformPath, opts) => {
+      (filename, localPth, transformPath, opts) => {
         const babelError = new SyntaxError(message);
 
         babelError.type = 'SyntaxError';

packages/metro/src/JSTransformer/worker.js

@@ -78,6 +78,7 @@ export type WorkerOptions = {|
   +dev: boolean,
   +hot: boolean,
   +inlineRequires: boolean,
+  +isScript: boolean,
   +minify: boolean,
   +platform: ?string,
   +projectRoot: string,
@@ -124,9 +125,7 @@ function getDynamicDepsBehavior(
 async function transformCode(
   filename: string,
   localPath: LocalPath,
-  sourceCode: ?string,
   transformerPath: string,
-  isScript: boolean,
   options: WorkerOptions,
   assetExts: $ReadOnlyArray<string>,
   assetRegistryPath: string,
@@ -142,17 +141,13 @@ async function transformCode(
     start_timestamp: process.hrtime(),
   };
 
-  let data;
+  const data = fs.readFileSync(filename);
+  const sourceCode = data.toString('utf8');
   let type = 'js/module';
 
-  if (sourceCode == null) {
-    data = fs.readFileSync(filename);
-    sourceCode = data.toString('utf8');
-  }
-
   const sha1 = crypto
     .createHash('sha1')
-    .update(data || sourceCode)
+    .update(data)
     .digest('hex');
 
   if (filename.endsWith('.json')) {
@@ -226,7 +221,7 @@ async function transformCode(
   // dependency graph and it code will just be prepended to the bundle modules),
   // we need to wrap it differently than a commonJS module (also, scripts do
   // not have dependencies).
-  if (isScript) {
+  if (options.isScript) {
     dependencies = [];
     wrappedAst = JsFileWrapping.wrapPolyfill(ast);
 

packages/metro/src/JSTransformer/worker/__tests__/__snapshots__/worker-test.js.snap

@@ -1,3 +1,3 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`code transformation worker: reports filename when encountering unsupported dynamic dependency 1`] = `"arbitrary/file.js:3:10: calls to \`require\` expect exactly 1 string literal argument, but this was found: \`require(a)\`."`;
+exports[`code transformation worker: reports filename when encountering unsupported dynamic dependency 1`] = `"/root/local/file.js:3:10: calls to \`require\` expect exactly 1 string literal argument, but this was found: \`require(a)\`."`;

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

@@ -21,19 +21,45 @@ jest
   .mock('metro-minify-uglify');
 
 const path = require('path');
-const transformCode = require('..').transform;
+
-const {InvalidRequireCallError} = require('..');
+const transformerPath = require.resolve('metro/src/transformer.js');
+const transformerContents = require('fs').readFileSync(transformerPath);
+
+const babelRcPath = require.resolve('metro/rn-babelrc.json');
+const babelRcContents = require('fs').readFileSync(babelRcPath);
+
+let fs;
+let mkdirp;
+let transformCode;
+let InvalidRequireCallError;
 
 describe('code transformation worker:', () => {
+  beforeEach(() => {
+    jest.resetModules();
+
+    jest.mock('fs', () => new (require('metro-memory-fs'))());
+
+    fs = require('fs');
+    mkdirp = require('mkdirp');
+    ({transform: transformCode, InvalidRequireCallError} = require('..'));
+    fs.reset();
+
+    mkdirp.sync('/root/local');
+    mkdirp.sync(path.dirname(transformerPath));
+    fs.writeFileSync(transformerPath, transformerContents);
+    fs.writeFileSync(babelRcPath, babelRcContents);
+  });
+
   it('transforms a simple script', async () => {
+    fs.writeFileSync('/root/local/file.js', 'someReallyArbitrary(code)');
+
     const {result} = await transformCode(
-      'arbitrary/file.js',
+      '/root/local/file.js',
       `local/file.js`,
-      'someReallyArbitrary(code)',
+      transformerPath,
-      require.resolve('metro/src/transformer.js'),
-      true,
       {
         dev: true,
+        isScript: true,
         transform: {},
       },
       [],
@@ -56,14 +82,15 @@ describe('code transformation worker:', () => {
   });
 
   it('transforms a simple module', async () => {
+    fs.writeFileSync('/root/local/file.js', 'arbitrary(code)');
+
     const {result} = await transformCode(
-      'arbitrary/file.js',
+      '/root/local/file.js',
       `local/file.js`,
-      'arbitrary(code)',
+      transformerPath,
-      require.resolve('metro/src/transformer.js'),
-      false,
       {
         dev: true,
+        isScript: false,
         transform: {},
       },
       [],
@@ -86,9 +113,8 @@ describe('code transformation worker:', () => {
   });
 
   it(`transforms a module with dependencies`, async () => {
-    const {result} = await transformCode(
+    fs.writeFileSync(
-      'arbitrary/file.js',
+      '/root/local/file.js',
-      `local/file.js`,
       [
         "'use strict';",
         'require("./a");',
@@ -96,8 +122,12 @@ describe('code transformation worker:', () => {
         'const b = require("b");',
         'import c from "./c";',
       ].join('\n'),
-      require.resolve('metro/src/transformer.js'),
+    );
-      false,
+
+    const {result} = await transformCode(
+      '/root/local/file.js',
+      `local/file.js`,
+      transformerPath,
       {
         dev: true,
         transform: {},
@@ -134,17 +164,20 @@ describe('code transformation worker:', () => {
   });
 
   it('reports filename when encountering unsupported dynamic dependency', async () => {
+    fs.writeFileSync(
+      '/root/local/file.js',
+      [
+        'require("./a");',
+        'let a = arbitrary(code);',
+        'const b = require(a);',
+      ].join('\n'),
+    );
+
     try {
       await transformCode(
-        'arbitrary/file.js',
+        '/root/local/file.js',
         `local/file.js`,
-        [
+        transformerPath,
-          'require("./a");',
-          'let a = arbitrary(code);',
-          'const b = require(a);',
-        ].join('\n'),
-        path.join(__dirname, '../../../transformer.js'),
-        false,
         {
           dev: true,
           transform: {},
@@ -165,12 +198,13 @@ describe('code transformation worker:', () => {
   });
 
   it('supports dynamic dependencies from within `node_modules`', async () => {
+    mkdirp.sync('/root/node_modules/foo');
+    fs.writeFileSync('/root/node_modules/foo/bar.js', 'require(foo.bar);');
+
     await transformCode(
-      '/root/node_modules/bar/file.js',
+      '/root/node_modules/foo/bar.js',
-      `node_modules/bar/file.js`,
+      `node_modules/foo/bar.js`,
-      'require(global.something);\n',
+      transformerPath,
-      path.join(__dirname, '../../../transformer.js'),
-      false,
       {
         dev: true,
         transform: {},
@@ -185,13 +219,13 @@ describe('code transformation worker:', () => {
   });
 
   it('minifies the code correctly', async () => {
+    fs.writeFileSync('/root/local/file.js', 'arbitrary(code);');
+
     expect(
       (await transformCode(
-        '/root/node_modules/bar/file.js',
+        '/root/local/file.js',
-        `node_modules/bar/file.js`,
+        `local/file.js`,
-        'arbitrary(code);',
+        transformerPath,
-        path.join(__dirname, '../../../transformer.js'),
-        false,
         {
           dev: true,
           minify: true,
@@ -214,13 +248,13 @@ describe('code transformation worker:', () => {
   });
 
   it('minifies a JSON file', async () => {
+    fs.writeFileSync('/root/local/file.json', 'arbitrary(code);');
+
     expect(
       (await transformCode(
-        '/root/node_modules/bar/file.json',
+        '/root/local/file.json',
-        `node_modules/bar/file.js`,
+        `local/file.json`,
-        'arbitrary(code);',
+        transformerPath,
-        path.join(__dirname, '../../../transformer.js'),
-        false,
         {
           dev: true,
           minify: true,

packages/metro/src/lib/transformHelpers.js

@@ -36,6 +36,7 @@ async function calcTransformerOptions(
     dev: options.dev,
     hot: options.hot,
     inlineRequires: false,
+    isScript: options.type === 'script',
     minify: options.minify,
     platform: options.platform,
     projectRoot,

packages/metro/src/node-haste/Module.js

@@ -24,7 +24,6 @@ type ReadResult = {
 
 export type TransformCode = (
   module: Module,
-  sourceCode: ?string,
   transformOptions: WorkerOptions,
 ) => Promise<TransformedCode>;
 
@@ -74,7 +73,6 @@ class Module {
   async read(transformOptions: WorkerOptions): Promise<ReadResult> {
     const result: TransformedCode = await this._transformCode(
       this,
-      null, // Source code is read on the worker
       transformOptions,
     );