From e04b3bf52e1e679fe1ef2a39b5d2b90f87f11d19 Mon Sep 17 00:00:00 2001 From: Jean Lauliac Date: Mon, 6 Feb 2017 06:54:45 -0800 Subject: [PATCH] packager: reestablish changes reverted by d63f9c Reviewed By: cpojer Differential Revision: D4514889 fbshipit-source-id: de5d1350cbcea7a26395e532fefd78a816167b4a --- packages/metro-bundler/src/Bundler/index.js | 5 +- .../__tests__/Transformer-test.js | 11 ++- .../metro-bundler/src/JSTransformer/index.js | 80 ++++--------------- 3 files changed, 24 insertions(+), 72 deletions(-) diff --git a/packages/metro-bundler/src/Bundler/index.js b/packages/metro-bundler/src/Bundler/index.js index dc701656..8f4832be 100644 --- a/packages/metro-bundler/src/Bundler/index.js +++ b/packages/metro-bundler/src/Bundler/index.js @@ -143,9 +143,8 @@ class Bundler { cacheKey: transformCacheKey, }); - this._transformer = new Transformer({ - transformModulePath: opts.transformModulePath, - }); + /* $FlowFixMe: in practice it's always here. */ + this._transformer = new Transformer(opts.transformModulePath); this._resolver = new Resolver({ assetExts: opts.assetExts, diff --git a/packages/metro-bundler/src/JSTransformer/__tests__/Transformer-test.js b/packages/metro-bundler/src/JSTransformer/__tests__/Transformer-test.js index 0df1a5f0..077b530f 100644 --- a/packages/metro-bundler/src/JSTransformer/__tests__/Transformer-test.js +++ b/packages/metro-bundler/src/JSTransformer/__tests__/Transformer-test.js @@ -25,7 +25,7 @@ var Transformer = require('../'); const {any} = jasmine; describe('Transformer', function() { - let options, workers, Cache; + let workers, Cache; const fileName = '/an/arbitrary/file.js'; const transformModulePath = __filename; @@ -34,7 +34,6 @@ describe('Transformer', function() { Cache.prototype.get = jest.fn((a, b, c) => c()); fs.writeFileSync.mockClear(); - options = {transformModulePath}; workerFarm.mockClear(); workerFarm.mockImplementation((opts, path, methods) => { const api = workers = {}; @@ -43,11 +42,11 @@ describe('Transformer', function() { }); }); - it('passes transform module path, file path, source code,' + - ' and options to the worker farm when transforming', () => { + it('passes transform module path, file path, source code' + + ' to the worker farm when transforming', () => { const transformOptions = {arbitrary: 'options'}; const code = 'arbitrary(code)'; - new Transformer(options).transformFile(fileName, code, transformOptions); + new Transformer(transformModulePath).transformFile(fileName, code, transformOptions); expect(workers.transformAndExtractDependencies).toBeCalledWith( transformModulePath, fileName, @@ -58,7 +57,7 @@ describe('Transformer', function() { }); it('should add file info to parse errors', function() { - const transformer = new Transformer(options); + const transformer = new Transformer(transformModulePath); var message = 'message'; var snippet = 'snippet'; diff --git a/packages/metro-bundler/src/JSTransformer/index.js b/packages/metro-bundler/src/JSTransformer/index.js index 9ebcf053..cab5bf7c 100644 --- a/packages/metro-bundler/src/JSTransformer/index.js +++ b/packages/metro-bundler/src/JSTransformer/index.js @@ -13,12 +13,13 @@ const Logger = require('../Logger'); -const declareOpts = require('../lib/declareOpts'); +const debug = require('debug')('RNP:JStransformer'); const denodeify = require('denodeify'); +const invariant = require('fbjs/lib/invariant'); const os = require('os'); +const path = require('path'); const util = require('util'); const workerFarm = require('worker-farm'); -const debug = require('debug')('RNP:JStransformer'); import type {Data as TransformData, Options as TransformOptions} from './worker/worker'; import type {SourceMap} from '../lib/SourceMap'; @@ -29,36 +30,11 @@ import type {SourceMap} from '../lib/SourceMap'; const MAX_CALLS_PER_WORKER = 600; // Worker will timeout if one of the callers timeout. -const DEFAULT_MAX_CALL_TIME = 301000; +const TRANSFORM_TIMEOUT_INTERVAL = 301000; // How may times can we tolerate failures from the worker. const MAX_RETRIES = 2; -const validateOpts = declareOpts({ - transformModulePath: { - type:'string', - required: false, - }, - transformTimeoutInterval: { - type: 'number', - default: DEFAULT_MAX_CALL_TIME, - }, - worker: { - type: 'string', - }, - methods: { - type: 'array', - default: [], - }, -}); - -type Options = { - transformModulePath?: ?string, - transformTimeoutInterval?: ?number, - worker?: ?string, - methods?: ?Array, -}; - const maxConcurrentWorkers = ((cores, override) => { if (override) { return Math.min(cores, override); @@ -93,14 +69,8 @@ function makeFarm(worker, methods, timeout) { class Transformer { - _opts: { - transformModulePath?: ?string, - transformTimeoutInterval: number, - worker: ?string, - methods: Array, - }; _workers: {[name: string]: mixed}; - _transformModulePath: ?string; + _transformModulePath: string; _transform: ( transform: string, filename: string, @@ -113,31 +83,17 @@ class Transformer { sourceMap: SourceMap, ) => Promise<{code: string, map: SourceMap}>; - constructor(options: Options) { - const opts = this._opts = validateOpts(options); + constructor(transformModulePath: string) { + invariant(path.isAbsolute(transformModulePath), 'transform module path should be absolute'); + this._transformModulePath = transformModulePath; - const {transformModulePath} = opts; - - if (opts.worker) { - this._workers = - makeFarm(opts.worker, opts.methods, opts.transformTimeoutInterval); - opts.methods.forEach(name => { - /* $FlowFixMe: assigning the class object fields directly is - * questionable, because it's prone to conflicts. */ - this[name] = this._workers[name]; - }); - } - else if (transformModulePath) { - this._transformModulePath = require.resolve(transformModulePath); - - this._workers = makeFarm( - require.resolve('./worker'), - ['minify', 'transformAndExtractDependencies'], - opts.transformTimeoutInterval, - ); - this._transform = denodeify(this._workers.transformAndExtractDependencies); - this.minify = denodeify(this._workers.minify); - } + this._workers = makeFarm( + require.resolve('./worker'), + ['minify', 'transformAndExtractDependencies'], + TRANSFORM_TIMEOUT_INTERVAL, + ); + this._transform = denodeify(this._workers.transformAndExtractDependencies); + this.minify = denodeify(this._workers.minify); } kill() { @@ -150,7 +106,6 @@ class Transformer { } debug('transforming file', fileName); return this - /* $FlowFixMe: _transformModulePath may be empty, see constructor */ ._transform(this._transformModulePath, fileName, code, options) .then(data => { Logger.log(data.transformFileStartLogEntry); @@ -162,7 +117,7 @@ class Transformer { if (error.type === 'TimeoutError') { const timeoutErr = new Error( `TimeoutError: transforming ${fileName} took longer than ` + - `${this._opts.transformTimeoutInterval / 1000} seconds.\n` + + `${TRANSFORM_TIMEOUT_INTERVAL / 1000} seconds.\n` + 'You can adjust timeout via the \'transformTimeoutInterval\' option' ); /* $FlowFixMe: monkey-patch Error */ @@ -171,8 +126,7 @@ class Transformer { } else if (error.type === 'ProcessTerminatedError') { const uncaughtError = new Error( 'Uncaught error in the transformer worker: ' + - /* $FlowFixMe: _transformModulePath may be empty, see constructor */ - this._opts.transformModulePath + this._transformModulePath ); /* $FlowFixMe: monkey-patch Error */ uncaughtError.type = 'ProcessTerminatedError';