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
This commit is contained in:
Jean Lauliac 2016-12-21 03:39:47 -08:00 committed by Facebook Github Bot
parent fd5af61e5c
commit b1132a91ab
4 changed files with 37 additions and 74 deletions

View File

@ -21,6 +21,7 @@ const workerFarm = require('worker-farm');
const debug = require('debug')('ReactNativePackager:JStransformer'); const debug = require('debug')('ReactNativePackager:JStransformer');
import type {Data as TransformData, Options as TransformOptions} from './worker/worker'; 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 // 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. // to avoid any memory leak while not slowing down initial builds.
@ -109,8 +110,8 @@ class Transformer {
minify: ( minify: (
filename: string, filename: string,
code: string, code: string,
sourceMap: string, sourceMap: SourceMap,
) => Promise<mixed>; ) => Promise<{code: string, map: SourceMap}>;
constructor(options: Options) { constructor(options: Options) {
const opts = this._opts = validateOpts(options); const opts = this._opts = validateOpts(options);

View File

@ -22,60 +22,27 @@ import type Module from '../node-haste/Module';
import type {SourceMap} from '../lib/SourceMap'; import type {SourceMap} from '../lib/SourceMap';
import type {Options as TransformOptions} from '../JSTransformer/worker/worker'; import type {Options as TransformOptions} from '../JSTransformer/worker/worker';
import type {Reporter} from '../lib/reporting'; import type {Reporter} from '../lib/reporting';
import type {TransformCode} from '../node-haste/Module';
import type Cache from '../node-haste/Cache';
const validateOpts = declareOpts({ type MinifyCode = (filePath: string, code: string, map: SourceMap) =>
projectRoots: { Promise<{code: string, map: SourceMap}>;
type: 'array',
required: true, type Options = {
}, assetExts: Array<string>,
blacklistRE: { blacklistRE?: RegExp,
type: 'object', // typeof regex is object cache: Cache,
}, extraNodeModules?: {},
polyfillModuleNames: { minifyCode: MinifyCode,
type: 'array', platforms: Array<string>,
default: [], polyfillModuleNames?: Array<string>,
}, projectRoots: Array<string>,
moduleFormat: { reporter: Reporter,
type: 'string', resetCache: boolean,
default: 'haste', transformCacheKey: string,
}, transformCode: TransformCode,
watch: { watch?: boolean,
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',
},
});
const getDependenciesValidateOpts = declareOpts({ const getDependenciesValidateOpts = declareOpts({
dev: { dev: {
@ -99,16 +66,10 @@ const getDependenciesValidateOpts = declareOpts({
class Resolver { class Resolver {
_depGraph: DependencyGraph; _depGraph: DependencyGraph;
_minifyCode: (filePath: string, code: string, map: SourceMap) => _minifyCode: MinifyCode;
Promise<{code: string, map: SourceMap}>;
_polyfillModuleNames: Array<string>; _polyfillModuleNames: Array<string>;
constructor(options: { constructor(opts: Options) {
reporter: Reporter,
resetCache: boolean,
}) {
const opts = validateOpts(options);
this._depGraph = new DependencyGraph({ this._depGraph = new DependencyGraph({
assetDependencies: ['react-native/Libraries/Image/AssetRegistry'], assetDependencies: ['react-native/Libraries/Image/AssetRegistry'],
assetExts: opts.assetExts, assetExts: opts.assetExts,
@ -116,21 +77,21 @@ class Resolver {
extraNodeModules: opts.extraNodeModules, extraNodeModules: opts.extraNodeModules,
ignoreFilePath: function(filepath) { ignoreFilePath: function(filepath) {
return filepath.indexOf('__tests__') !== -1 || return filepath.indexOf('__tests__') !== -1 ||
(opts.blacklistRE && opts.blacklistRE.test(filepath)); (opts.blacklistRE != null && opts.blacklistRE.test(filepath));
}, },
moduleOptions: { moduleOptions: {
cacheTransformResults: true, cacheTransformResults: true,
resetCache: options.resetCache, resetCache: opts.resetCache,
}, },
platforms: opts.platforms, platforms: opts.platforms,
preferNativePlatform: true, preferNativePlatform: true,
providesModuleNodeModules: defaults.providesModuleNodeModules, providesModuleNodeModules: defaults.providesModuleNodeModules,
reporter: options.reporter, reporter: opts.reporter,
resetCache: options.resetCache, resetCache: opts.resetCache,
roots: opts.projectRoots, roots: opts.projectRoots,
transformCacheKey: opts.transformCacheKey, transformCacheKey: opts.transformCacheKey,
transformCode: opts.transformCode, transformCode: opts.transformCode,
watch: opts.watch, watch: opts.watch || false,
}); });
this._minifyCode = opts.minifyCode; this._minifyCode = opts.minifyCode;

View File

@ -32,7 +32,7 @@ type DirExistsFn = (filePath: string) => boolean;
type Options = { type Options = {
dirExists: DirExistsFn, dirExists: DirExistsFn,
entryPath: string, entryPath: string,
extraNodeModules: Object, extraNodeModules: ?Object,
hasteFS: HasteFS, hasteFS: HasteFS,
hasteMap: HasteMap, hasteMap: HasteMap,
helpers: DependencyGraphHelpers, helpers: DependencyGraphHelpers,
@ -46,7 +46,7 @@ type Options = {
class ResolutionRequest { class ResolutionRequest {
_dirExists: DirExistsFn; _dirExists: DirExistsFn;
_entryPath: string; _entryPath: string;
_extraNodeModules: Object; _extraNodeModules: ?Object;
_hasteFS: HasteFS; _hasteFS: HasteFS;
_hasteMap: HasteMap; _hasteMap: HasteMap;
_helpers: DependencyGraphHelpers; _helpers: DependencyGraphHelpers;
@ -338,10 +338,11 @@ class ResolutionRequest {
} }
if (this._extraNodeModules) { if (this._extraNodeModules) {
const {_extraNodeModules} = this;
const bits = toModuleName.split('/'); const bits = toModuleName.split('/');
const packageName = bits[0]; const packageName = bits[0];
if (this._extraNodeModules[packageName]) { if (_extraNodeModules[packageName]) {
bits[0] = this._extraNodeModules[packageName]; bits[0] = _extraNodeModules[packageName];
searchQueue.push(path.join.apply(path, bits)); searchQueue.push(path.join.apply(path, bits));
} }
} }

View File

@ -51,7 +51,7 @@ class DependencyGraph {
_opts: {| _opts: {|
assetExts: Array<string>, assetExts: Array<string>,
extensions: Array<string>, extensions: Array<string>,
extraNodeModules: Object, extraNodeModules: ?Object,
forceNodeFilesystemAPI: boolean, forceNodeFilesystemAPI: boolean,
ignoreFilePath: (filePath: string) => boolean, ignoreFilePath: (filePath: string) => boolean,
maxWorkers: ?number, maxWorkers: ?number,
@ -107,7 +107,7 @@ class DependencyGraph {
assetExts: Array<string>, assetExts: Array<string>,
cache: Cache, cache: Cache,
extensions?: ?Array<string>, extensions?: ?Array<string>,
extraNodeModules: Object, extraNodeModules: ?Object,
forceNodeFilesystemAPI?: boolean, forceNodeFilesystemAPI?: boolean,
ignoreFilePath: (filePath: string) => boolean, ignoreFilePath: (filePath: string) => boolean,
maxWorkers?: ?number, maxWorkers?: ?number,