From 2517d18535c8172727ac890271fe0c2557ef456f Mon Sep 17 00:00:00 2001 From: Christoph Pojer Date: Thu, 9 Feb 2017 03:59:30 -0800 Subject: [PATCH] Remove baseTransformer Reviewed By: jeanlauliac Differential Revision: D4506124 fbshipit-source-id: 642f06dffe4ea710113e8e8426915bf1b40d4611 --- .../worker/__tests__/worker-test.js | 88 ++++++------- .../src/JSTransformer/worker/worker.js | 121 ++++++++---------- .../src/ModuleGraph/types.flow.js | 24 ++-- .../worker/__tests__/optimize-module-test.js | 4 +- .../worker/__tests__/transform-module-test.js | 28 ++-- .../ModuleGraph/worker/transform-module.js | 26 ++-- packages/metro-bundler/transformer.js | 13 -- 7 files changed, 143 insertions(+), 161 deletions(-) diff --git a/packages/metro-bundler/src/JSTransformer/worker/__tests__/worker-test.js b/packages/metro-bundler/src/JSTransformer/worker/__tests__/worker-test.js index 4421b3a1..5a9b8f08 100644 --- a/packages/metro-bundler/src/JSTransformer/worker/__tests__/worker-test.js +++ b/packages/metro-bundler/src/JSTransformer/worker/__tests__/worker-test.js @@ -14,35 +14,46 @@ jest.mock('../extract-dependencies'); jest.mock('../inline'); jest.mock('../minify'); -const {any, objectContaining} = jasmine; +const {objectContaining} = jasmine; describe('code transformation worker:', () => { let transformCode; - let extractDependencies, transform; + let extractDependencies, transformer; beforeEach(() => { jest.resetModules(); ({transformCode} = require('..')); extractDependencies = require('../extract-dependencies').mockReturnValue({}); - transform = jest.fn(); + transformer = { + transform: jest.fn((src, filename, options) => ({ + code: src, + map: {}, + })), + }; }); it('calls the transform with file name, source code, and transform options', function() { const filename = 'arbitrary/file.js'; const sourceCode = 'arbitrary(code)'; const transformOptions = {arbitrary: 'options'}; - transformCode(transform, filename, sourceCode, {transform: transformOptions}); - expect(transform).toBeCalledWith( - {filename, sourceCode, options: transformOptions}, any(Function)); + transformCode(transformer, filename, sourceCode, {transform: transformOptions}, () => {}); + expect(transformer.transform).toBeCalledWith( + sourceCode, + filename, + transformOptions, + ); }); it('prefixes JSON files with an assignment to module.exports to make the code valid', function() { const filename = 'arbitrary/file.json'; const sourceCode = '{"arbitrary":"property"}'; - transformCode(transform, filename, sourceCode, {}); - expect(transform).toBeCalledWith( - {filename, sourceCode: `module.exports=${sourceCode}`}, any(Function)); + transformCode(transformer, filename, sourceCode, {}, () => {}); + expect(transformer.transform).toBeCalledWith( + `module.exports=${sourceCode}`, + filename, + undefined, + ); }); it('calls back with the result of the transform in the cache', done => { @@ -50,10 +61,8 @@ describe('code transformation worker:', () => { code: 'some.other(code)', map: {} }; - transform.mockImplementation((_, callback) => - callback(null, result)); - transformCode(transform, 'filename', 'code', {}, (error, data) => { + transformCode(transformer, 'filename', result.code, {}, (error, data) => { expect(error).toBeNull(); expect(data.result).toEqual(objectContaining(result)); done(); @@ -64,14 +73,11 @@ describe('code transformation worker:', () => { 'removes the leading assignment to `module.exports` before passing ' + 'on the result if the file is a JSON file, even if minified', done => { - const result = { - code: 'p.exports={a:1,b:2}', - }; - transform.mockImplementation((_, callback) => callback(null, result)); + const code = '{a:1,b:2}'; const filePath = 'arbitrary/file.json'; - transformCode(transform, filePath, 'b', {}, (error, data) => { + transformCode(transformer, filePath, code, {}, (error, data) => { expect(error).toBeNull(); - expect(data.result.code).toEqual('{a:1,b:2}'); + expect(data.result.code).toEqual(code); done(); }, ); @@ -83,9 +89,8 @@ describe('code transformation worker:', () => { const result = { code: `${shebang} \n arbitrary(code)`, }; - transform.mockImplementation((_, callback) => callback(null, result)); const filePath = 'arbitrary/file.js'; - transformCode(transform, filePath, 'b', {}, (error, data) => { + transformCode(transformer, filePath, result.code, {}, (error, data) => { expect(error).toBeNull(); const {code} = data.result; expect(code).not.toContain(shebang); @@ -95,26 +100,22 @@ describe('code transformation worker:', () => { }); it('calls back with any error yielded by the transform', done => { - const error = Error('arbitrary error'); - transform.mockImplementation((_, callback) => callback(error)); - transformCode(transform, 'filename', 'code', {}, e => { - expect(e).toBe(error); + const message = 'SyntaxError: this code is broken.'; + transformer.transform.mockImplementation(() => { + throw new Error(message); + }); + + transformCode(transformer, 'filename', 'code', {}, error => { + expect(error.message).toBe(message); done(); }); }); - describe('dependency extraction:', () => { - let code; - - beforeEach(() => { - transform.mockImplementation( - (_, callback) => callback(null, {code})); - }); - + describe('dependency extraction', () => { it('passes the transformed code the `extractDependencies`', done => { - code = 'arbitrary(code)'; + const code = 'arbitrary(code)'; - transformCode(transform, 'filename', 'code', {}, (error) => { + transformCode(transformer, 'filename', code, {}, (error) => { expect(error).toBeNull(); expect(extractDependencies).toBeCalledWith(code); done(); @@ -131,7 +132,7 @@ describe('code transformation worker:', () => { }; extractDependencies.mockReturnValue(dependencyData); - transformCode(transform, 'filename', 'code', {}, (error, data) => { + transformCode(transformer, 'filename', 'code', {}, (error, data) => { expect(error).toBeNull(); expect(data.result).toEqual(objectContaining(dependencyData)); done(); @@ -141,7 +142,7 @@ describe('code transformation worker:', () => { it('does not extract requires if files are marked as "extern"', done => { const opts = {extern: true}; - transformCode(transform, 'filename', 'code', opts, (error, data) => { + transformCode(transformer, 'filename', 'code', opts, (error, data) => { expect(error).toBeNull(); const {dependencies, dependencyOffsets} = data.result; expect(extractDependencies).not.toBeCalled(); @@ -153,7 +154,7 @@ describe('code transformation worker:', () => { it('does not extract requires of JSON files', done => { const jsonStr = '{"arbitrary":"json"}'; - transformCode(transform, 'arbitrary.json', jsonStr, {}, (error, data) => { + transformCode(transformer, 'arbitrary.json', jsonStr, {}, (error, data) => { expect(error).toBeNull(); const {dependencies, dependencyOffsets} = data.result; expect(extractDependencies).not.toBeCalled(); @@ -187,13 +188,12 @@ describe('code transformation worker:', () => { extractDependencies.mockImplementation( code => code === foldedCode ? dependencyData : {}); - transform.mockImplementation( - (_, callback) => callback(null, transformResult)); + transformer.transform.mockImplementation((src, fileName, _) => transformResult); }); it('passes the transform result to `inline` for constant inlining', done => { transformResult = {map: {version: 3}, code: 'arbitrary(code)'}; - transformCode(transform, filename, 'code', options, () => { + transformCode(transformer, filename, 'code', options, () => { expect(inline).toBeCalledWith(filename, transformResult, options); done(); }); @@ -202,21 +202,21 @@ describe('code transformation worker:', () => { it('passes the result obtained from `inline` on to `constant-folding`', done => { const inlineResult = {map: {version: 3, sources: []}, ast: {}}; inline.mockReturnValue(inlineResult); - transformCode(transform, filename, 'code', options, () => { + transformCode(transformer, filename, 'code', options, () => { expect(constantFolding).toBeCalledWith(filename, inlineResult); done(); }); }); it('Uses the code obtained from `constant-folding` to extract dependencies', done => { - transformCode(transform, filename, 'code', options, () => { + transformCode(transformer, filename, 'code', options, () => { expect(extractDependencies).toBeCalledWith(foldedCode); done(); }); }); it('uses the dependencies obtained from the optimized result', done => { - transformCode(transform, filename, 'code', options, (_, data) => { + transformCode(transformer, filename, 'code', options, (_, data) => { const result = data.result; expect(result.dependencies).toEqual(dependencyData.dependencies); done(); @@ -224,7 +224,7 @@ describe('code transformation worker:', () => { }); it('uses data produced by `constant-folding` for the result', done => { - transformCode(transform, 'filename', 'code', options, (_, data) => { + transformCode(transformer, 'filename', 'code', options, (_, data) => { expect(data.result) .toEqual(objectContaining({code: foldedCode, map: foldedMap})); done(); diff --git a/packages/metro-bundler/src/JSTransformer/worker/worker.js b/packages/metro-bundler/src/JSTransformer/worker/worker.js index d97714dc..52b77718 100644 --- a/packages/metro-bundler/src/JSTransformer/worker/worker.js +++ b/packages/metro-bundler/src/JSTransformer/worker/worker.js @@ -20,20 +20,6 @@ const minify = require('./minify'); import type {LogEntry} from '../../Logger/Types'; import type {Ast, SourceMap, TransformOptions as BabelTransformOptions} from 'babel-core'; -function makeTransformParams(filename, sourceCode, options, willMinify) { - invariant( - !willMinify || options.generateSourceMaps, - 'Minifying source code requires the `generateSourceMaps` option to be `true`', - ); - - - if (filename.endsWith('.json')) { - sourceCode = 'module.exports=' + sourceCode; - } - - return {filename, sourceCode, options}; -} - export type TransformedCode = { code: string, dependencies: Array, @@ -41,17 +27,13 @@ export type TransformedCode = { map?: ?SourceMap, }; -type Transform = ( - params: { +type Transformer = { + transform: ( filename: string, sourceCode: string, options: ?{}, - }, - callback: ( - error?: Error, - tranformed?: {ast: ?Ast, code: string, map: ?SourceMap}, - ) => mixed, -) => void; + ) => {ast: ?Ast, code: string, map: ?SourceMap} +}; export type TransformOptions = { generateSourceMaps: boolean, @@ -80,19 +62,21 @@ type Callback = ( ) => mixed; function transformCode( - transform: Transform, + transformer: Transformer, filename: string, sourceCode: string, options: Options, callback: Callback, ) { - const params = makeTransformParams( - filename, - sourceCode, - options.transform, - options.minify, + invariant( + !options.minify || options.transform.generateSourceMaps, + 'Minifying source code requires the `generateSourceMaps` option to be `true`', ); + const isJson = filename.endsWith('.json'); + if (isJson) { + sourceCode = 'module.exports=' + sourceCode; + } const transformFileStartLogEntry = { action_name: 'Transforming file', @@ -102,52 +86,53 @@ function transformCode( start_timestamp: process.hrtime(), }; - transform(params, (error, transformed) => { - if (error) { - callback(error); - return; - } + let transformed; + try { + transformed = transformer.transform(sourceCode, filename, options.transform); + } catch (error) { + callback(error); + return; + } - invariant( - transformed != null, - 'Missing transform results despite having no error.', - ); + invariant( + transformed != null, + 'Missing transform results despite having no error.', + ); - var code, map; - if (options.minify) { - ({code, map} = - constantFolding(filename, inline(filename, transformed, options))); - invariant(code != null, 'Missing code from constant-folding transform.'); - } else { - ({code, map} = transformed); - } + var code, map; + if (options.minify) { + ({code, map} = + constantFolding(filename, inline(filename, transformed, options))); + invariant(code != null, 'Missing code from constant-folding transform.'); + } else { + ({code, map} = transformed); + } - if (isJson) { - code = code.replace(/^\w+\.exports=/, ''); - } else { - // Remove shebang - code = code.replace(/^#!.*/, ''); - } + if (isJson) { + code = code.replace(/^\w+\.exports=/, ''); + } else { + // Remove shebang + code = code.replace(/^#!.*/, ''); + } - const depsResult = isJson || options.extern - ? {dependencies: [], dependencyOffsets: []} - : extractDependencies(code); + const depsResult = isJson || options.extern + ? {dependencies: [], dependencyOffsets: []} + : extractDependencies(code); - const timeDelta = process.hrtime(transformFileStartLogEntry.start_timestamp); - const duration_ms = Math.round((timeDelta[0] * 1e9 + timeDelta[1]) / 1e6); - const transformFileEndLogEntry = { - action_name: 'Transforming file', - action_phase: 'end', - file_name: filename, - duration_ms: duration_ms, - log_entry_label: 'Transforming file', - }; + const timeDelta = process.hrtime(transformFileStartLogEntry.start_timestamp); + const duration_ms = Math.round((timeDelta[0] * 1e9 + timeDelta[1]) / 1e6); + const transformFileEndLogEntry = { + action_name: 'Transforming file', + action_phase: 'end', + file_name: filename, + duration_ms: duration_ms, + log_entry_label: 'Transforming file', + }; - return callback(null, { - result: {...depsResult, code, map}, - transformFileStartLogEntry, - transformFileEndLogEntry, - }); + callback(null, { + result: {...depsResult, code, map}, + transformFileStartLogEntry, + transformFileEndLogEntry, }); } diff --git a/packages/metro-bundler/src/ModuleGraph/types.flow.js b/packages/metro-bundler/src/ModuleGraph/types.flow.js index fabb48f2..2c0c038c 100644 --- a/packages/metro-bundler/src/ModuleGraph/types.flow.js +++ b/packages/metro-bundler/src/ModuleGraph/types.flow.js @@ -11,6 +11,7 @@ 'use strict'; import type {SourceMap} from './output/source-map'; +import type {Ast} from 'babel-core'; import type {Console} from 'console'; export type Callback @@ -99,18 +100,19 @@ type ResolveOptions = { log?: Console, }; -export type TransformFn = ( - data: {| - filename: string, - options?: Object, - plugins?: Array, - sourceCode: string, - |}, - callback: Callback -) => void; +export type TransformerResult = { + ast: ?Ast, + code: string, + map: ?SourceMap, +}; -export type TransformFnResult = { - ast: Object, +export type Transformer = { + transform: ( + sourceCode: string, + filename: string, + options: ?{}, + plugins?: Array, + ) => {ast: ?Ast, code: string, map: ?SourceMap} }; export type TransformResult = {| diff --git a/packages/metro-bundler/src/ModuleGraph/worker/__tests__/optimize-module-test.js b/packages/metro-bundler/src/ModuleGraph/worker/__tests__/optimize-module-test.js index c9c3cc6b..de120e8f 100644 --- a/packages/metro-bundler/src/ModuleGraph/worker/__tests__/optimize-module-test.js +++ b/packages/metro-bundler/src/ModuleGraph/worker/__tests__/optimize-module-test.js @@ -12,7 +12,7 @@ jest.disableAutomock(); const optimizeModule = require('../optimize-module'); const transformModule = require('../transform-module'); -const transform = require('../../../../transformer.js'); +const transformer = require('../../../../transformer.js'); const {SourceMapConsumer} = require('source-map'); const {objectContaining} = jasmine; @@ -32,7 +32,7 @@ describe('optimizing JS modules', () => { let transformResult; beforeAll(done => { - transformModule(originalCode, {filename, transform}, (error, result) => { + transformModule(originalCode, {filename, transformer}, (error, result) => { if (error) { throw error; } diff --git a/packages/metro-bundler/src/ModuleGraph/worker/__tests__/transform-module-test.js b/packages/metro-bundler/src/ModuleGraph/worker/__tests__/transform-module-test.js index 6c912822..65e5b232 100644 --- a/packages/metro-bundler/src/ModuleGraph/worker/__tests__/transform-module-test.js +++ b/packages/metro-bundler/src/ModuleGraph/worker/__tests__/transform-module-test.js @@ -24,18 +24,20 @@ const {any, objectContaining} = jasmine; describe('transforming JS modules:', () => { const filename = 'arbitrary'; - let transform; + let transformer; beforeEach(() => { - transform = fn(); - transform.stub.yields(null, transformResult()); + transformer = { + transform: fn(), + }; + transformer.transform.stub.returns(transformResult()); }); const {bodyAst, sourceCode, transformedCode} = createTestData(); const options = variants => ({ filename, - transform, + transformer, variants, }); @@ -80,17 +82,17 @@ describe('transforming JS modules:', () => { const variants = {dev: {dev: true}, prod: {dev: false}}; transformModule(sourceCode, options(variants), () => { - expect(transform) - .toBeCalledWith({filename, sourceCode, options: variants.dev}, any(Function)); - expect(transform) - .toBeCalledWith({filename, sourceCode, options: variants.prod}, any(Function)); + expect(transformer.transform) + .toBeCalledWith(sourceCode, filename, variants.dev); + expect(transformer.transform) + .toBeCalledWith(sourceCode, filename, variants.prod); done(); }); }); it('calls back with any error yielded by the transform function', done => { const error = new Error(); - transform.stub.yields(error); + transformer.transform.stub.throws(error); transformModule(sourceCode, options(), e => { expect(e).toBe(error); @@ -138,7 +140,7 @@ describe('transforming JS modules:', () => { const dep1 = 'foo', dep2 = 'bar'; const code = `require('${dep1}'),require('${dep2}')`; const {body} = parse(code).program; - transform.stub.yields(null, transformResult(body)); + transformer.transform.stub.returns(transformResult(body)); transformModule(code, options(), (error, result) => { expect(result.transformed.default) @@ -149,11 +151,11 @@ describe('transforming JS modules:', () => { it('transforms for all variants', done => { const variants = {dev: {dev: true}, prod: {dev: false}}; - transform.stub + transformer.transform.stub .withArgs(filename, sourceCode, variants.dev) - .yields(null, transformResult(bodyAst)) + .returns(transformResult(bodyAst)) .withArgs(filename, sourceCode, variants.prod) - .yields(null, transformResult([])); + .returns(transformResult([])); transformModule(sourceCode, options(variants), (error, result) => { const {dev, prod} = result.transformed; diff --git a/packages/metro-bundler/src/ModuleGraph/worker/transform-module.js b/packages/metro-bundler/src/ModuleGraph/worker/transform-module.js index bc079b0f..9f670e4c 100644 --- a/packages/metro-bundler/src/ModuleGraph/worker/transform-module.js +++ b/packages/metro-bundler/src/ModuleGraph/worker/transform-module.js @@ -21,8 +21,8 @@ const {basename} = require('path'); import type { Callback, TransformedFile, - TransformFn, - TransformFnResult, + Transformer, + TransformerResult, TransformResult, TransformVariants, } from '../types.flow'; @@ -30,7 +30,7 @@ import type { export type TransformOptions = {| filename: string, polyfill?: boolean, - transform: TransformFn, + transformer: Transformer, variants?: TransformVariants, |}; @@ -47,17 +47,23 @@ function transformModule( return transformJSON(code, options, callback); } - const {filename, transform, variants = defaultVariants} = options; + const {filename, transformer, variants = defaultVariants} = options; const tasks = {}; Object.keys(variants).forEach(name => { - tasks[name] = cb => transform({ - filename, - sourceCode: code, - options: variants[name], - }, cb); + tasks[name] = cb => { + try { + cb(null, transformer.transform( + code, + filename, + variants[name], + )); + } catch (error) { + cb(error, null); + } + }; }); - series(tasks, (error, results: {[key: string]: TransformFnResult}) => { + series(tasks, (error, results: {[key: string]: TransformerResult}) => { if (error) { callback(error); return; diff --git a/packages/metro-bundler/transformer.js b/packages/metro-bundler/transformer.js index 5b45d15c..f60b6e5a 100644 --- a/packages/metro-bundler/transformer.js +++ b/packages/metro-bundler/transformer.js @@ -126,17 +126,4 @@ function transform(src, filename, options) { } } -module.exports = function(data, callback) { - let result; - try { - result = transform(data.sourceCode, data.filename, data.options); - } catch (e) { - callback(e); - return; - } - - callback(null, result); -}; - -// export for use in jest module.exports.transform = transform;