From 23261a674571769f97587a30be377512afaffc23 Mon Sep 17 00:00:00 2001 From: Jean Lauliac Date: Wed, 21 Dec 2016 03:39:47 -0800 Subject: [PATCH] packager: Fix typing of Resolver options Summary: `declareOpts` is weakly typed. Since the callsite of Resolver constructor is itself flowifyed, we can get rid of `declareOpts`, and this provides us with much better typing. Eventually I'd like to get rid of most of the defaults in the packager's inner code (ex. `watch` being false, `dev` being true). The reason is that defaults everywhere are prone to causing inconsistencies (for ex. some other code could have `dev` as false by default), problems that cannot be caught by Flow. Instead of having non-required options, I believe it is more sensible to provide helper functions that build sets of default options. Reviewed By: davidaurelio, cpojer Differential Revision: D4351874 fbshipit-source-id: 38653063f8939b4282c7c27cb6d5e3f3a25a9484 --- react-packager/src/JSTransformer/index.js | 5 +- react-packager/src/Resolver/index.js | 93 ++++++------------- .../DependencyGraph/ResolutionRequest.js | 9 +- react-packager/src/node-haste/index.js | 4 +- 4 files changed, 37 insertions(+), 74 deletions(-) diff --git a/react-packager/src/JSTransformer/index.js b/react-packager/src/JSTransformer/index.js index ad4b7df2..b2337dd7 100644 --- a/react-packager/src/JSTransformer/index.js +++ b/react-packager/src/JSTransformer/index.js @@ -21,6 +21,7 @@ const workerFarm = require('worker-farm'); const debug = require('debug')('ReactNativePackager:JStransformer'); import type {Data as TransformData, Options as TransformOptions} from './worker/worker'; +import type {SourceMap} from '../lib/SourceMap'; // Avoid memory leaks caused in workers. This number seems to be a good enough number // to avoid any memory leak while not slowing down initial builds. @@ -109,8 +110,8 @@ class Transformer { minify: ( filename: string, code: string, - sourceMap: string, - ) => Promise; + sourceMap: SourceMap, + ) => Promise<{code: string, map: SourceMap}>; constructor(options: Options) { const opts = this._opts = validateOpts(options); diff --git a/react-packager/src/Resolver/index.js b/react-packager/src/Resolver/index.js index 0283498d..31e99496 100644 --- a/react-packager/src/Resolver/index.js +++ b/react-packager/src/Resolver/index.js @@ -22,60 +22,27 @@ import type Module from '../node-haste/Module'; import type {SourceMap} from '../lib/SourceMap'; import type {Options as TransformOptions} from '../JSTransformer/worker/worker'; import type {Reporter} from '../lib/reporting'; +import type {TransformCode} from '../node-haste/Module'; +import type Cache from '../node-haste/Cache'; -const validateOpts = declareOpts({ - projectRoots: { - type: 'array', - required: true, - }, - blacklistRE: { - type: 'object', // typeof regex is object - }, - polyfillModuleNames: { - type: 'array', - default: [], - }, - moduleFormat: { - type: 'string', - default: 'haste', - }, - watch: { - type: 'boolean', - default: false, - }, - assetExts: { - type: 'array', - required: true, - }, - platforms: { - type: 'array', - required: true, - }, - cache: { - type: 'object', - required: true, - }, - transformCode: { - type: 'function', - }, - transformCacheKey: { - type: 'string', - }, - extraNodeModules: { - type: 'object', - required: false, - }, - minifyCode: { - type: 'function', - }, - resetCache: { - type: 'boolean', - default: false, - }, - reporter: { - type: 'object', - }, -}); +type MinifyCode = (filePath: string, code: string, map: SourceMap) => + Promise<{code: string, map: SourceMap}>; + +type Options = { + assetExts: Array, + blacklistRE?: RegExp, + cache: Cache, + extraNodeModules?: {}, + minifyCode: MinifyCode, + platforms: Array, + polyfillModuleNames?: Array, + projectRoots: Array, + reporter: Reporter, + resetCache: boolean, + transformCacheKey: string, + transformCode: TransformCode, + watch?: boolean, +}; const getDependenciesValidateOpts = declareOpts({ dev: { @@ -99,16 +66,10 @@ const getDependenciesValidateOpts = declareOpts({ class Resolver { _depGraph: DependencyGraph; - _minifyCode: (filePath: string, code: string, map: SourceMap) => - Promise<{code: string, map: SourceMap}>; + _minifyCode: MinifyCode; _polyfillModuleNames: Array; - constructor(options: { - reporter: Reporter, - resetCache: boolean, - }) { - const opts = validateOpts(options); - + constructor(opts: Options) { this._depGraph = new DependencyGraph({ assetDependencies: ['react-native/Libraries/Image/AssetRegistry'], assetExts: opts.assetExts, @@ -116,21 +77,21 @@ class Resolver { extraNodeModules: opts.extraNodeModules, ignoreFilePath: function(filepath) { return filepath.indexOf('__tests__') !== -1 || - (opts.blacklistRE && opts.blacklistRE.test(filepath)); + (opts.blacklistRE != null && opts.blacklistRE.test(filepath)); }, moduleOptions: { cacheTransformResults: true, - resetCache: options.resetCache, + resetCache: opts.resetCache, }, platforms: opts.platforms, preferNativePlatform: true, providesModuleNodeModules: defaults.providesModuleNodeModules, - reporter: options.reporter, - resetCache: options.resetCache, + reporter: opts.reporter, + resetCache: opts.resetCache, roots: opts.projectRoots, transformCacheKey: opts.transformCacheKey, transformCode: opts.transformCode, - watch: opts.watch, + watch: opts.watch || false, }); this._minifyCode = opts.minifyCode; diff --git a/react-packager/src/node-haste/DependencyGraph/ResolutionRequest.js b/react-packager/src/node-haste/DependencyGraph/ResolutionRequest.js index ec4b67df..7c9a7721 100644 --- a/react-packager/src/node-haste/DependencyGraph/ResolutionRequest.js +++ b/react-packager/src/node-haste/DependencyGraph/ResolutionRequest.js @@ -32,7 +32,7 @@ type DirExistsFn = (filePath: string) => boolean; type Options = { dirExists: DirExistsFn, entryPath: string, - extraNodeModules: Object, + extraNodeModules: ?Object, hasteFS: HasteFS, hasteMap: HasteMap, helpers: DependencyGraphHelpers, @@ -46,7 +46,7 @@ type Options = { class ResolutionRequest { _dirExists: DirExistsFn; _entryPath: string; - _extraNodeModules: Object; + _extraNodeModules: ?Object; _hasteFS: HasteFS; _hasteMap: HasteMap; _helpers: DependencyGraphHelpers; @@ -338,10 +338,11 @@ class ResolutionRequest { } if (this._extraNodeModules) { + const {_extraNodeModules} = this; const bits = toModuleName.split('/'); const packageName = bits[0]; - if (this._extraNodeModules[packageName]) { - bits[0] = this._extraNodeModules[packageName]; + if (_extraNodeModules[packageName]) { + bits[0] = _extraNodeModules[packageName]; searchQueue.push(path.join.apply(path, bits)); } } diff --git a/react-packager/src/node-haste/index.js b/react-packager/src/node-haste/index.js index be128dee..13ded6d5 100644 --- a/react-packager/src/node-haste/index.js +++ b/react-packager/src/node-haste/index.js @@ -51,7 +51,7 @@ class DependencyGraph { _opts: {| assetExts: Array, extensions: Array, - extraNodeModules: Object, + extraNodeModules: ?Object, forceNodeFilesystemAPI: boolean, ignoreFilePath: (filePath: string) => boolean, maxWorkers: ?number, @@ -107,7 +107,7 @@ class DependencyGraph { assetExts: Array, cache: Cache, extensions?: ?Array, - extraNodeModules: Object, + extraNodeModules: ?Object, forceNodeFilesystemAPI?: boolean, ignoreFilePath: (filePath: string) => boolean, maxWorkers?: ?number,