Stronger typing for transformers

Reviewed By: jeanlauliac

Differential Revision: D5006679

fbshipit-source-id: 795c60db363fb53bc74697e4befe50995e9b97a7

flow/babel.js.flow

@@ -51,13 +51,14 @@ type GeneratorOptions = {
 };
 
 type InlinePlugin = string | {} | () => {};
+type _Plugins = Array<string | Object | [InlinePlugin] | [InlinePlugin, mixed]>;
 
 // based on https://babeljs.io/docs/usage/options/ -- 2016-11-11
 type __TransformOptions = {
   filename?: string,
   filenameRelative?: string,
   presets?: Array<string | Object>,
-  plugins?: Array<string | Object | [InlinePlugin] | [InlinePlugin, mixed]>,
+  plugins?: _Plugins,
   parserOpts?: BabylonOptions,
   generatorOpts?: GeneratorOptions,
   highlightCode?: boolean,
@@ -92,11 +93,13 @@ declare class _Ast {}
 type TransformResult = {
   ast: _Ast,
   code: ?string,
+  ignored: boolean,
   map: ?_SourceMap,
 };
 type VisitFn = <State>(path: Object, state: State) => any;
 
 declare module 'babel-core' {
+  declare type Plugins = _Plugins;
   declare type SourceMap = _SourceMap;
   declare type Ast = _Ast;
   declare type TransformOptions = _TransformOptions;

jest/preprocessor.js

@@ -5,6 +5,8 @@
  * This source code is licensed under the BSD-style license found in the
  * 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.
+ *
+ * @flow
  */
 'use strict';
 
@@ -24,7 +26,7 @@ babelRegisterOnly([]);
 const transformer = require('../packager/transformer.js');
 
 module.exports = {
-  process(src, file) {
+  process(src/*: string*/, file/*: string*/) {
     if (nodeFiles.test(file)) { // node specific transforms only
       return babel.transform(
         src,
@@ -32,7 +34,12 @@ module.exports = {
       ).code;
     }
 
-    return transformer.transform(src, file, {inlineRequires: true}).code;
+    return transformer.transform(src, file, {
+      dev: true,
+      inlineRequires: true,
+      platform: '',
+      projectRoot: '',
+    }).code;
   },
 
   getCacheKey: createCacheKeyFunction([

packager/src/Bundler/index.js

@@ -138,6 +138,8 @@ type Options = {|
   +watch: boolean,
 |};
 
+const {hasOwnProperty} = Object;
+
 class Bundler {
 
   _opts: Options;
@@ -673,7 +675,7 @@ class Bundler {
       const preloaded =
         module.path === entryFilePath ||
         isPolyfill ||
-        preloadedModules && preloadedModules.hasOwnProperty(module.path);
+        preloadedModules && hasOwnProperty.call(preloadedModules, module.path);
 
       return new ModuleTransport({
         name,

packager/src/JSTransformer/index.js

@@ -20,7 +20,7 @@ const path = require('path');
 const util = require('util');
 const workerFarm = require('../worker-farm');
 
-import type {Data as TransformData, Options as TransformOptions} from './worker/worker';
+import type {Data as TransformData, Options as WorkerOptions} from './worker/worker';
 import type {MappingsMap} from '../lib/SourceMap';
 
 // Avoid memory leaks caused in workers. This number seems to be a good enough number
@@ -63,7 +63,7 @@ class Transformer {
     transform: string,
     filename: string,
     sourceCode: string,
-    options: ?TransformOptions,
+    options: ?WorkerOptions,
   ) => Promise<TransformData>;
   minify: (
     filename: string,
@@ -97,7 +97,7 @@ class Transformer {
     this._workers && workerFarm.end(this._workers);
   }
 
-  transformFile(fileName: string, code: string, options: TransformOptions) {
+  transformFile(fileName: string, code: string, options: WorkerOptions) {
     if (!this._transform) {
       return Promise.reject(new Error('No transform module'));
     }

packager/src/JSTransformer/worker/worker.js

@@ -11,6 +11,7 @@
 
 'use strict';
 
+const babelRegisterOnly = require('../../../babelRegisterOnly');
 const constantFolding = require('./constant-folding');
 const extractDependencies = require('./extract-dependencies');
 const inline = require('./inline');
@@ -18,7 +19,7 @@ const invariant = require('fbjs/lib/invariant');
 const minify = require('./minify');
 
 import type {LogEntry} from '../../Logger/Types';
-import type {Ast, SourceMap as MappingsMap} from 'babel-core';
+import type {Ast, Plugins as BabelPlugins, SourceMap as MappingsMap} from 'babel-core';
 
 export type TransformedCode = {
   code: string,
@@ -27,15 +28,18 @@ export type TransformedCode = {
   map?: ?MappingsMap,
 };
 
-type Transformer = {
+export type Transformer<ExtraOptions: {} = {}> = {
   transform: (
     filename: string,
     sourceCode: string,
-    options: ?{},
-  ) => {ast: ?Ast, code: string, map: ?MappingsMap}
+    options: ExtraOptions & TransformOptions,
+    plugins?: BabelPlugins,
+  ) => {ast: ?Ast, code: string, map: ?MappingsMap},
+  getCacheKey: TransformOptionsStrict => string,
 };
 
-export type TransformOptions = {|
+
+export type TransformOptionsStrict = {|
   +dev: boolean,
   +generateSourceMaps: boolean,
   +hot: boolean,
@@ -44,11 +48,20 @@ export type TransformOptions = {|
   +projectRoot: string,
 |};
 
+export type TransformOptions = {
+  +dev?: boolean,
+  +generateSourceMaps?: boolean,
+  +hot?: boolean,
+  +inlineRequires?: {+blacklist: {[string]: true}} | boolean,
+  +platform: string,
+  +projectRoot: string,
+};
+
 export type Options = {|
   +dev: boolean,
   +minify: boolean,
   +platform: string,
-  +transform: TransformOptions,
+  +transform: TransformOptionsStrict,
 |};
 
 export type Data = {
@@ -63,7 +76,7 @@ type Callback = (
 ) => mixed;
 
 function transformCode(
-  transformer: Transformer,
+  transformer: Transformer<*>,
   filename: string,
   sourceCode: string,
   options: Options,
@@ -144,8 +157,9 @@ exports.transformAndExtractDependencies = (
   options: Options,
   callback: Callback,
 ) => {
+  babelRegisterOnly([transform]);
   /* $FlowFixMe: impossible to type a dynamic require */
-  const transformModule = require(transform);
+  const transformModule: Transformer<*> = require(transform);
   transformCode(transformModule, filename, sourceCode, options, callback);
 };
 

packager/src/ModuleGraph/types.flow.js

@@ -13,6 +13,7 @@
 import type {MappingsMap, SourceMap} from '../lib/SourceMap';
 import type {Ast} from 'babel-core';
 import type {Console} from 'console';
+export type {Transformer} from '../JSTransformer/worker/worker.js';
 
 export type Callback<A = void, B = void>
   = (Error => void)
@@ -105,15 +106,6 @@ export type TransformerResult = {|
   map: ?MappingsMap,
 |};
 
-export type Transformer = {
-  transform: (
-    sourceCode: string,
-    filename: string,
-    options: ?{},
-    plugins?: Array<string | Object | [string | Object, any]>,
-  ) => {ast: ?Ast, code: string, map: ?MappingsMap}
-};
-
 export type TransformResult = {|
   code: string,
   dependencies: Array<string>,

packager/src/ModuleGraph/worker/__tests__/transform-module-test.js

@@ -80,6 +80,15 @@ describe('transforming JS modules:', () => {
     });
   });
 
+  const defaults = {
+    dev: false,
+    generateSourceMaps: true,
+    hot: false,
+    inlineRequires: false,
+    platform: '',
+    projectRoot: '',
+  };
+
   it('calls the passed-in transform function with code, file name, and options ' +
     'for all passed in variants',
     done => {
@@ -87,9 +96,9 @@ describe('transforming JS modules:', () => {
 
       transformModule(sourceCode, options(variants), () => {
         expect(transformer.transform)
-          .toBeCalledWith(sourceCode, filename, variants.dev);
+          .toBeCalledWith(sourceCode, filename, {...defaults, ...variants.dev});
         expect(transformer.transform)
-          .toBeCalledWith(sourceCode, filename, variants.prod);
+          .toBeCalledWith(sourceCode, filename, {...defaults, ...variants.prod});
         done();
       });
     },

packager/src/ModuleGraph/worker/transform-module.js

@@ -12,6 +12,7 @@
 
 const JsFileWrapping = require('./JsFileWrapping');
 
+const asyncify = require('async/asyncify');
 const collectDependencies = require('./collect-dependencies');
 const defaults = require('../../../defaults');
 const docblock = require('../../node-haste/DependencyGraph/docblock');
@@ -34,10 +35,18 @@ import type {
 export type TransformOptions = {|
   filename: string,
   polyfill?: boolean,
-  transformer: Transformer,
+  transformer: Transformer<*>,
   variants?: TransformVariants,
 |};
 
+const defaultTransformOptions = {
+  dev: true,
+  generateSourceMaps: true,
+  hot: false,
+  inlineRequires: false,
+  platform: '',
+  projectRoot: '',
+};
 const defaultVariants = {default: {}};
 
 const ASSET_EXTENSIONS = new Set(defaults.assetExts);
@@ -61,17 +70,12 @@ function transformModule(
   const {filename, transformer, variants = defaultVariants} = options;
   const tasks = {};
   Object.keys(variants).forEach(name => {
-    tasks[name] = cb => {
-      try {
-        cb(null, transformer.transform(
-          code,
-          filename,
-          variants[name],
-        ));
-      } catch (error) {
-        cb(error, null);
-      }
-    };
+    tasks[name] = asyncify(() => transformer.transform(
+        code,
+        filename,
+        {...defaultTransformOptions, ...variants[name]},
+      )
+    );
   });
 
   series(tasks, (error, results: {[key: string]: TransformerResult}) => {

packager/src/lib/GlobalTransformCache.js

@@ -24,7 +24,7 @@ const throat = require('throat');
 
 import type {
   Options as TransformWorkerOptions,
-  TransformOptions,
+  TransformOptionsStrict,
 } from '../JSTransformer/worker/worker';
 import type {CachedResult, GetTransformCacheKey} from './TransformCache';
 
@@ -381,7 +381,7 @@ class OptionsHasher {
    * of the cache key as they should not affect the transformation of a single
    * particular file.
    */
-  hashTransformOptions(hash: crypto$Hash, options: TransformOptions): crypto$Hash {
+  hashTransformOptions(hash: crypto$Hash, options: TransformOptionsStrict): crypto$Hash {
     const {
       generateSourceMaps, dev, hot, inlineRequires, platform, projectRoot,
       ...unknowns,

packager/src/lib/TransformCache.js

@@ -20,7 +20,7 @@ const rimraf = require('rimraf');
 const terminal = require('../lib/terminal');
 const writeFileAtomicSync = require('write-file-atomic').sync;
 
-import type {Options as TransformOptions} from '../JSTransformer/worker/worker';
+import type {Options as WorkerOptions} from '../JSTransformer/worker/worker';
 import type {MappingsMap} from './SourceMap';
 import type {Reporter} from './reporting';
 
@@ -58,7 +58,7 @@ function hashSourceCode(props: {
   filePath: string,
   sourceCode: string,
   getTransformCacheKey: GetTransformCacheKey,
-  transformOptions: TransformOptions,
+  transformOptions: WorkerOptions,
   transformOptionsKey: string,
 }): string {
   return crypto.createHash('sha1')
@@ -134,7 +134,7 @@ function writeSync(props: {
   filePath: string,
   sourceCode: string,
   getTransformCacheKey: GetTransformCacheKey,
-  transformOptions: TransformOptions,
+  transformOptions: WorkerOptions,
   transformOptionsKey: string,
   result: CachedResult,
 }): void {
@@ -326,7 +326,7 @@ function readMetadataFileSync(
 export type ReadTransformProps = {
   filePath: string,
   sourceCode: string,
-  transformOptions: TransformOptions,
+  transformOptions: WorkerOptions,
   transformOptionsKey: string,
   getTransformCacheKey: GetTransformCacheKey,
   cacheOptions: CacheOptions,

packager/src/node-haste/Module.js

@@ -22,7 +22,7 @@ const jsonStableStringify = require('json-stable-stringify');
 
 const {join: joinPath, relative: relativePath, extname} = require('path');
 
-import type {TransformedCode, Options as TransformOptions} from '../JSTransformer/worker/worker';
+import type {TransformedCode, Options as WorkerOptions} from '../JSTransformer/worker/worker';
 import type {GlobalTransformCache} from '../lib/GlobalTransformCache';
 import type {MappingsMap} from '../lib/SourceMap';
 import type {GetTransformCacheKey} from '../lib/TransformCache';
@@ -47,7 +47,7 @@ export type CachedReadResult = {|
 export type TransformCode = (
   module: Module,
   sourceCode: string,
-  transformOptions: TransformOptions,
+  transformOptions: WorkerOptions,
 ) => Promise<TransformedCode>;
 
 export type HasteImpl = {
@@ -131,11 +131,11 @@ class Module {
     return Promise.resolve().then(() => this._getHasteName() != null);
   }
 
-  getCode(transformOptions: TransformOptions) {
+  getCode(transformOptions: WorkerOptions) {
     return this.read(transformOptions).then(({code}) => code);
   }
 
-  getMap(transformOptions: TransformOptions) {
+  getMap(transformOptions: WorkerOptions) {
     return this.read(transformOptions).then(({map}) => map);
   }
 
@@ -168,7 +168,7 @@ class Module {
     return this._moduleCache.getPackageForModule(this);
   }
 
-  getDependencies(transformOptions: TransformOptions) {
+  getDependencies(transformOptions: WorkerOptions) {
     return this.read(transformOptions).then(({dependencies}) => dependencies);
   }
 
@@ -324,7 +324,7 @@ class Module {
    * Shorthand for reading both from cache or from fresh for all call sites that
    * are asynchronous by default.
    */
-  read(transformOptions: TransformOptions): Promise<ReadResult> {
+  read(transformOptions: WorkerOptions): Promise<ReadResult> {
     return Promise.resolve().then(() => {
       const cached = this.readCached(transformOptions);
       if (cached.result != null) {
@@ -339,7 +339,7 @@ class Module {
    * the file from source. This has the benefit of being synchronous. As a
    * result it is possible to read many cached Module in a row, synchronously.
    */
-  readCached(transformOptions: TransformOptions): CachedReadResult {
+  readCached(transformOptions: WorkerOptions): CachedReadResult {
     const key = stableObjectHash(transformOptions || {});
     let result = this._readResultsByOptionsKey.get(key);
     if (result != null) {
@@ -355,7 +355,7 @@ class Module {
    * so it's faster in case the results are already in memory.
    */
   _readFromTransformCache(
-    transformOptions: TransformOptions,
+    transformOptions: WorkerOptions,
     transformOptionsKey: string,
   ): CachedReadResult {
     const cacheProps = this._getCacheProps(transformOptions, transformOptionsKey);
@@ -375,7 +375,7 @@ class Module {
    * scratch. We don't repeat the same work as `readCached` because we assume
    * call sites have called it already.
    */
-  readFresh(transformOptions: TransformOptions): Promise<ReadResult> {
+  readFresh(transformOptions: WorkerOptions): Promise<ReadResult> {
     const key = stableObjectHash(transformOptions || {});
     const promise = this._readPromises.get(key);
     if (promise != null) {
@@ -404,7 +404,7 @@ class Module {
     return freshPromise;
   }
 
-  _getCacheProps(transformOptions: TransformOptions, transformOptionsKey: string) {
+  _getCacheProps(transformOptions: WorkerOptions, transformOptionsKey: string) {
     const sourceCode = this._readSourceCode();
     const getTransformCacheKey = this._getTransformCacheKey;
     return {

packager/transformer.js

@@ -7,6 +7,8 @@
  * of patent rights can be found in the PATENTS file in the same directory.
  *
  * Note: This is a fork of the fb-specific transform.js
+ *
+ * @flow
  */
 'use strict';
 
@@ -23,6 +25,9 @@ const resolvePlugins = require('babel-preset-react-native/lib/resolvePlugins');
 
 const {compactMapping} = require('./src/Bundler/source-map');
 
+import type {Plugins as BabelPlugins} from 'babel-core';
+import type {Transformer, TransformOptions} from './src/JSTransformer/worker/worker';
+
 const cacheKeyParts = [
   fs.readFileSync(__filename),
   require('babel-plugin-external-helpers/package.json').version,
@@ -36,14 +41,14 @@ const cacheKeyParts = [
  * default RN babelrc file and uses that.
  */
 const getBabelRC = (function() {
-  let babelRC = null;
+  let babelRC: ?{extends?: string, plugins: BabelPlugins} = null;
 
   return function _getBabelRC(projectRoot) {
     if (babelRC !== null) {
       return babelRC;
     }
 
-    babelRC = {plugins: []}; // empty babelrc
+    babelRC = {plugins: []};
 
     // Let's look for the .babelrc in the project root.
     // In the future let's look into adding a command line option to specify
@@ -62,6 +67,7 @@ const getBabelRC = (function() {
         );
 
       // Require the babel-preset's listed in the default babel config
+      // $FlowFixMe: dynamic require can't be avoided
       babelRC.presets = babelRC.presets.map(preset => require('babel-preset-' + preset));
       babelRC.plugins = resolvePlugins(babelRC.plugins);
     } else {
@@ -91,7 +97,7 @@ function buildBabelConfig(filename, options) {
   const extraPlugins = [externalHelpersPlugin];
 
   var inlineRequires = options.inlineRequires;
-  var blacklist = inlineRequires && inlineRequires.blacklist;
+  var blacklist = typeof inlineRequires === 'object' ? inlineRequires.blacklist : null;
   if (inlineRequires && !(blacklist && filename in blacklist)) {
     extraPlugins.push(inlineRequiresPlugin);
   }
@@ -106,7 +112,11 @@ function buildBabelConfig(filename, options) {
   return Object.assign({}, babelRC, config);
 }
 
-function transform(src, filename, options) {
+function transform(
+  src: string,
+  filename: string,
+  options,
+) {
   options = options || {};
 
   const OLD_BABEL_ENV = process.env.BABEL_ENV;
@@ -144,13 +154,13 @@ function transform(src, filename, options) {
   }
 }
 
-function getCacheKey(options) {
+function getCacheKey(options: TransformOptions) {
   var key = crypto.createHash('md5');
   cacheKeyParts.forEach(part => key.update(part));
   return key.digest('hex');
 }
 
-module.exports = {
+module.exports = ({
   transform,
   getCacheKey,
-};
+}: Transformer<>);