packager: JSTransform cleanup: remove options object

Summary: We can simplify as there's only one option, and it seems to me we can also make it required, as otherwise the `JSTransformer` is essentially doing nothing. I don't believe we still have use cases where no transform happens at all, even in OSS?

Reviewed By: davidaurelio

Differential Revision: D4481723

fbshipit-source-id: b2e3830b206d56242d298ff3a7b5f4587ecfd29a
This commit is contained in:
Jean Lauliac 2017-01-31 05:25:42 -08:00 committed by Facebook Github Bot
parent f8c72f5441
commit fd1a1325f3
3 changed files with 22 additions and 44 deletions

View File

@ -143,9 +143,8 @@ class Bundler {
cacheKey: transformCacheKey, cacheKey: transformCacheKey,
}); });
this._transformer = new Transformer({ /* $FlowFixMe: in practice it's always here. */
transformModulePath: opts.transformModulePath, this._transformer = new Transformer(opts.transformModulePath);
});
this._resolver = new Resolver({ this._resolver = new Resolver({
assetExts: opts.assetExts, assetExts: opts.assetExts,

View File

@ -25,7 +25,7 @@ var Transformer = require('../');
const {any} = jasmine; const {any} = jasmine;
describe('Transformer', function() { describe('Transformer', function() {
let options, workers, Cache; let workers, Cache;
const fileName = '/an/arbitrary/file.js'; const fileName = '/an/arbitrary/file.js';
const transformModulePath = __filename; const transformModulePath = __filename;
@ -34,7 +34,6 @@ describe('Transformer', function() {
Cache.prototype.get = jest.fn((a, b, c) => c()); Cache.prototype.get = jest.fn((a, b, c) => c());
fs.writeFileSync.mockClear(); fs.writeFileSync.mockClear();
options = {transformModulePath};
workerFarm.mockClear(); workerFarm.mockClear();
workerFarm.mockImplementation((opts, path, methods) => { workerFarm.mockImplementation((opts, path, methods) => {
const api = workers = {}; const api = workers = {};
@ -43,11 +42,11 @@ describe('Transformer', function() {
}); });
}); });
it('passes transform module path, file path, source code,' + it('passes transform module path, file path, source code' +
' and options to the worker farm when transforming', () => { ' to the worker farm when transforming', () => {
const transformOptions = {arbitrary: 'options'}; const transformOptions = {arbitrary: 'options'};
const code = 'arbitrary(code)'; const code = 'arbitrary(code)';
new Transformer(options).transformFile(fileName, code, transformOptions); new Transformer(transformModulePath).transformFile(fileName, code, transformOptions);
expect(workers.transformAndExtractDependencies).toBeCalledWith( expect(workers.transformAndExtractDependencies).toBeCalledWith(
transformModulePath, transformModulePath,
fileName, fileName,
@ -58,7 +57,7 @@ describe('Transformer', function() {
}); });
it('should add file info to parse errors', function() { it('should add file info to parse errors', function() {
const transformer = new Transformer(options); const transformer = new Transformer(transformModulePath);
var message = 'message'; var message = 'message';
var snippet = 'snippet'; var snippet = 'snippet';

View File

@ -13,12 +13,13 @@
const Logger = require('../Logger'); const Logger = require('../Logger');
const declareOpts = require('../lib/declareOpts'); const debug = require('debug')('RNP:JStransformer');
const denodeify = require('denodeify'); const denodeify = require('denodeify');
const invariant = require('fbjs/lib/invariant');
const os = require('os'); const os = require('os');
const path = require('path');
const util = require('util'); const util = require('util');
const workerFarm = require('worker-farm'); const workerFarm = require('worker-farm');
const debug = require('debug')('RNP: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'; import type {SourceMap} from '../lib/SourceMap';
@ -34,17 +35,6 @@ const TRANSFORM_TIMEOUT_INTERVAL = 301000;
// How may times can we tolerate failures from the worker. // How may times can we tolerate failures from the worker.
const MAX_RETRIES = 2; const MAX_RETRIES = 2;
const validateOpts = declareOpts({
transformModulePath: {
type:'string',
required: false,
},
});
type Options = {
transformModulePath?: ?string,
};
const maxConcurrentWorkers = ((cores, override) => { const maxConcurrentWorkers = ((cores, override) => {
if (override) { if (override) {
return Math.min(cores, override); return Math.min(cores, override);
@ -79,11 +69,8 @@ function makeFarm(worker, methods, timeout) {
class Transformer { class Transformer {
_opts: {
transformModulePath?: ?string,
};
_workers: {[name: string]: mixed}; _workers: {[name: string]: mixed};
_transformModulePath: ?string; _transformModulePath: string;
_transform: ( _transform: (
transform: string, transform: string,
filename: string, filename: string,
@ -96,22 +83,17 @@ class Transformer {
sourceMap: SourceMap, sourceMap: SourceMap,
) => Promise<{code: string, map: SourceMap}>; ) => Promise<{code: string, map: SourceMap}>;
constructor(options: Options) { constructor(transformModulePath: string) {
const opts = this._opts = validateOpts(options); invariant(path.isAbsolute(transformModulePath), 'transform module path should be absolute');
this._transformModulePath = transformModulePath;
const {transformModulePath} = opts; this._workers = makeFarm(
require.resolve('./worker'),
if (transformModulePath) { ['minify', 'transformAndExtractDependencies'],
this._transformModulePath = require.resolve(transformModulePath); TRANSFORM_TIMEOUT_INTERVAL,
);
this._workers = makeFarm( this._transform = denodeify(this._workers.transformAndExtractDependencies);
require.resolve('./worker'), this.minify = denodeify(this._workers.minify);
['minify', 'transformAndExtractDependencies'],
TRANSFORM_TIMEOUT_INTERVAL,
);
this._transform = denodeify(this._workers.transformAndExtractDependencies);
this.minify = denodeify(this._workers.minify);
}
} }
kill() { kill() {
@ -124,7 +106,6 @@ class Transformer {
} }
debug('transforming file', fileName); debug('transforming file', fileName);
return this return this
/* $FlowFixMe: _transformModulePath may be empty, see constructor */
._transform(this._transformModulePath, fileName, code, options) ._transform(this._transformModulePath, fileName, code, options)
.then(data => { .then(data => {
Logger.log(data.transformFileStartLogEntry); Logger.log(data.transformFileStartLogEntry);
@ -145,8 +126,7 @@ class Transformer {
} else if (error.type === 'ProcessTerminatedError') { } else if (error.type === 'ProcessTerminatedError') {
const uncaughtError = new Error( const uncaughtError = new Error(
'Uncaught error in the transformer worker: ' + 'Uncaught error in the transformer worker: ' +
/* $FlowFixMe: _transformModulePath may be empty, see constructor */ this._transformModulePath
this._opts.transformModulePath
); );
/* $FlowFixMe: monkey-patch Error */ /* $FlowFixMe: monkey-patch Error */
uncaughtError.type = 'ProcessTerminatedError'; uncaughtError.type = 'ProcessTerminatedError';