From 2fbe3b799e775b07991ac240c5a10c1cffa2dee5 Mon Sep 17 00:00:00 2001 From: David Aurelio Date: Thu, 11 May 2017 16:37:07 -0700 Subject: [PATCH] Pass local paths to transformers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Pass `localPath` to transformers to allow usage of plugins like `transform-react-jsx-source` that don’t conflict with x-machine caches, e.g. Buck or our own global cache. These caches don’t work properly if paths local to a specific machine are part of cached outputs. Reviewed By: jeanlauliac Differential Revision: D5044147 fbshipit-source-id: 1177afb48d6dd9351738fcd4da8f9f6357e25e81 --- jest/preprocessor.js | 1 + packager/package.json | 2 +- packager/src/Bundler/index.js | 1 + .../__tests__/Transformer-test.js | 9 +++-- packager/src/JSTransformer/index.js | 36 +++++++++++++++---- .../worker/__tests__/worker-test.js | 32 +++++++++-------- packager/src/JSTransformer/worker/worker.js | 19 ++++++---- .../worker/__tests__/transform-module-test.js | 2 ++ .../ModuleGraph/worker/transform-module.js | 1 + 9 files changed, 71 insertions(+), 32 deletions(-) diff --git a/jest/preprocessor.js b/jest/preprocessor.js index d0c0582be09..2be2ec13ce4 100644 --- a/jest/preprocessor.js +++ b/jest/preprocessor.js @@ -36,6 +36,7 @@ module.exports = { return transformer.transform({ filename: file, + localPath: file, options: { dev: true, inlineRequires: true, diff --git a/packager/package.json b/packager/package.json index e04a905ad3b..66c609d0ef9 100644 --- a/packager/package.json +++ b/packager/package.json @@ -1,5 +1,5 @@ { - "version": "0.6.0", + "version": "0.6.1", "name": "react-native-packager", "description": "Build native apps with React!", "repository": { diff --git a/packager/src/Bundler/index.js b/packager/src/Bundler/index.js index 8619479af30..43142909c66 100644 --- a/packager/src/Bundler/index.js +++ b/packager/src/Bundler/index.js @@ -224,6 +224,7 @@ class Bundler { transformCode: (module, code, transformCodeOptions) => this._transformer.transformFile( module.path, + module.localPath, code, transformCodeOptions, ), diff --git a/packager/src/JSTransformer/__tests__/Transformer-test.js b/packager/src/JSTransformer/__tests__/Transformer-test.js index ae9bcf32d2e..ae9b59b13f6 100644 --- a/packager/src/JSTransformer/__tests__/Transformer-test.js +++ b/packager/src/JSTransformer/__tests__/Transformer-test.js @@ -30,6 +30,7 @@ const {Readable} = require('stream'); describe('Transformer', function() { let workers, Cache; const fileName = '/an/arbitrary/file.js'; + const localPath = 'arbitrary/file.js'; const transformModulePath = __filename; beforeEach(function() { @@ -49,10 +50,11 @@ describe('Transformer', function() { ' to the worker farm when transforming', () => { const transformOptions = {arbitrary: 'options'}; const code = 'arbitrary(code)'; - new Transformer(transformModulePath).transformFile(fileName, code, transformOptions); + new Transformer(transformModulePath).transformFile(fileName, localPath, code, transformOptions); expect(workers.transformAndExtractDependencies).toBeCalledWith( transformModulePath, fileName, + localPath, code, transformOptions, any(Function), @@ -65,7 +67,7 @@ describe('Transformer', function() { var snippet = 'snippet'; workers.transformAndExtractDependencies.mockImplementation( - function(transformPath, filename, code, opts, callback) { + function(transformPath, filename, localPth, code, opts, callback) { var babelError = new SyntaxError(message); babelError.type = 'SyntaxError'; babelError.description = message; @@ -78,7 +80,8 @@ describe('Transformer', function() { }, ); - return transformer.transformFile(fileName, '', {}) + expect.assertions(7); + return transformer.transformFile(fileName, localPath, '', {}) .catch(function(error) { expect(error.type).toEqual('TransformError'); expect(error.message).toBe('SyntaxError ' + message); diff --git a/packager/src/JSTransformer/index.js b/packager/src/JSTransformer/index.js index 7ca48881087..7b23fd1f4a9 100644 --- a/packager/src/JSTransformer/index.js +++ b/packager/src/JSTransformer/index.js @@ -14,14 +14,21 @@ const Logger = require('../Logger'); const debug = require('debug')('RNP:JStransformer'); -const denodeify = require('denodeify'); +const denodeify: Denodeify = require('denodeify'); const invariant = require('fbjs/lib/invariant'); const path = require('path'); const util = require('util'); const workerFarm = require('../worker-farm'); import type {Data as TransformData, Options as WorkerOptions} from './worker/worker'; +import type {LocalPath} from '../node-haste/lib/toLocalPath'; import type {MappingsMap} from '../lib/SourceMap'; +import typeof {minify as Minify, transformAndExtractDependencies as TransformAndExtractDependencies} from './worker/worker'; + +type CB = (?Error, ?T) => mixed; +type Denodeify = + & (((A, B, C, CB) => void) => (A, B, C) => Promise) + & (((A, B, C, D, E, CB) => void) => (A, B, C, D, E) => Promise); // 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. @@ -62,8 +69,9 @@ class Transformer { _transform: ( transform: string, filename: string, + localPath: LocalPath, sourceCode: string, - options: ?WorkerOptions, + options: WorkerOptions, ) => Promise; minify: ( filename: string, @@ -71,7 +79,11 @@ class Transformer { sourceMap: MappingsMap, ) => Promise<{code: string, map: MappingsMap}>; - constructor(transformModulePath: string, maxWorkerCount: number, reporters: Reporters) { + constructor( + transformModulePath: string, + maxWorkerCount: number, + reporters: Reporters, + ) { invariant(path.isAbsolute(transformModulePath), 'transform module path should be absolute'); this._transformModulePath = transformModulePath; @@ -89,21 +101,31 @@ class Transformer { }); this._workers = farm.methods; - this._transform = denodeify(this._workers.transformAndExtractDependencies); - this.minify = denodeify(this._workers.minify); + this._transform = denodeify((this._workers.transformAndExtractDependencies: TransformAndExtractDependencies)); + this.minify = denodeify((this._workers.minify: Minify)); } kill() { this._workers && workerFarm.end(this._workers); } - transformFile(fileName: string, code: string, options: WorkerOptions) { + transformFile( + fileName: string, + localPath: LocalPath, + code: string, + options: WorkerOptions) { if (!this._transform) { return Promise.reject(new Error('No transform module')); } debug('transforming file', fileName); return this - ._transform(this._transformModulePath, fileName, code, options) + ._transform( + this._transformModulePath, + fileName, + localPath, + code, + options, + ) .then(data => { Logger.log(data.transformFileStartLogEntry); Logger.log(data.transformFileEndLogEntry); diff --git a/packager/src/JSTransformer/worker/__tests__/worker-test.js b/packager/src/JSTransformer/worker/__tests__/worker-test.js index 262deeac80d..c404aea5d7e 100644 --- a/packager/src/JSTransformer/worker/__tests__/worker-test.js +++ b/packager/src/JSTransformer/worker/__tests__/worker-test.js @@ -35,11 +35,13 @@ describe('code transformation worker:', () => { it('calls the transform with file name, source code, and transform options', function() { const filename = 'arbitrary/file.js'; + const localPath = `local/${filename}`; const sourceCode = 'arbitrary(code)'; const transformOptions = {arbitrary: 'options'}; - transformCode(transformer, filename, sourceCode, {transform: transformOptions}, () => {}); + transformCode(transformer, filename, localPath, sourceCode, {transform: transformOptions}, () => {}); expect(transformer.transform).toBeCalledWith({ filename, + localPath, options: transformOptions, src: sourceCode, }); @@ -47,10 +49,12 @@ describe('code transformation worker:', () => { it('prefixes JSON files with an assignment to module.exports to make the code valid', function() { const filename = 'arbitrary/file.json'; + const localPath = `local/${filename}`; const sourceCode = '{"arbitrary":"property"}'; - transformCode(transformer, filename, sourceCode, {}, () => {}); + transformCode(transformer, filename, localPath, sourceCode, {}, () => {}); expect(transformer.transform).toBeCalledWith({ filename, + localPath, options: undefined, src: `module.exports=${sourceCode}`, }); @@ -62,7 +66,7 @@ describe('code transformation worker:', () => { map: {}, }; - transformCode(transformer, 'filename', result.code, {}, (error, data) => { + transformCode(transformer, 'filename', 'local/filename', result.code, {}, (error, data) => { expect(error).toBeNull(); expect(data.result).toEqual(objectContaining(result)); done(); @@ -75,7 +79,7 @@ describe('code transformation worker:', () => { done => { const code = '{a:1,b:2}'; const filePath = 'arbitrary/file.json'; - transformCode(transformer, filePath, code, {}, (error, data) => { + transformCode(transformer, filePath, filePath, code, {}, (error, data) => { expect(error).toBeNull(); expect(data.result.code).toEqual(code); done(); @@ -90,7 +94,7 @@ describe('code transformation worker:', () => { code: `${shebang} \n arbitrary(code)`, }; const filePath = 'arbitrary/file.js'; - transformCode(transformer, filePath, result.code, {}, (error, data) => { + transformCode(transformer, filePath, filePath, result.code, {}, (error, data) => { expect(error).toBeNull(); const {code} = data.result; expect(code).not.toContain(shebang); @@ -105,7 +109,7 @@ describe('code transformation worker:', () => { throw new Error(message); }); - transformCode(transformer, 'filename', 'code', {}, error => { + transformCode(transformer, 'filename', 'local/filename', 'code', {}, error => { expect(error.message).toBe(message); done(); }); @@ -115,7 +119,7 @@ describe('code transformation worker:', () => { it('passes the transformed code the `extractDependencies`', done => { const code = 'arbitrary(code)'; - transformCode(transformer, 'filename', code, {}, error => { + transformCode(transformer, 'filename', 'local/filename', code, {}, error => { expect(error).toBeNull(); expect(extractDependencies).toBeCalledWith(code); done(); @@ -132,7 +136,7 @@ describe('code transformation worker:', () => { }; extractDependencies.mockReturnValue(dependencyData); - transformCode(transformer, 'filename', 'code', {}, (error, data) => { + transformCode(transformer, 'filename', 'local/filename', 'code', {}, (error, data) => { expect(error).toBeNull(); expect(data.result).toEqual(objectContaining(dependencyData)); done(); @@ -142,7 +146,7 @@ describe('code transformation worker:', () => { it('does not extract requires of JSON files', done => { const jsonStr = '{"arbitrary":"json"}'; - transformCode(transformer, 'arbitrary.json', jsonStr, {}, (error, data) => { + transformCode(transformer, 'arbitrary.json', 'local/arbitrary.json', jsonStr, {}, (error, data) => { expect(error).toBeNull(); const {dependencies, dependencyOffsets} = data.result; expect(extractDependencies).not.toBeCalled(); @@ -181,7 +185,7 @@ describe('code transformation worker:', () => { it('passes the transform result to `inline` for constant inlining', done => { transformResult = {map: {version: 3}, code: 'arbitrary(code)'}; - transformCode(transformer, filename, 'code', options, () => { + transformCode(transformer, filename, filename, 'code', options, () => { expect(inline).toBeCalledWith(filename, transformResult, options); done(); }); @@ -190,21 +194,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(transformer, filename, 'code', options, () => { + transformCode(transformer, filename, filename, 'code', options, () => { expect(constantFolding).toBeCalledWith(filename, inlineResult); done(); }); }); it('Uses the code obtained from `constant-folding` to extract dependencies', done => { - transformCode(transformer, filename, 'code', options, () => { + transformCode(transformer, filename, filename, 'code', options, () => { expect(extractDependencies).toBeCalledWith(foldedCode); done(); }); }); it('uses the dependencies obtained from the optimized result', done => { - transformCode(transformer, filename, 'code', options, (_, data) => { + transformCode(transformer, filename, filename, 'code', options, (_, data) => { const result = data.result; expect(result.dependencies).toEqual(dependencyData.dependencies); done(); @@ -212,7 +216,7 @@ describe('code transformation worker:', () => { }); it('uses data produced by `constant-folding` for the result', done => { - transformCode(transformer, 'filename', 'code', options, (_, data) => { + transformCode(transformer, 'filename', 'local/filename', 'code', options, (_, data) => { expect(data.result) .toEqual(objectContaining({code: foldedCode, map: foldedMap})); done(); diff --git a/packager/src/JSTransformer/worker/worker.js b/packager/src/JSTransformer/worker/worker.js index 9227e609d85..c7452e06b47 100644 --- a/packager/src/JSTransformer/worker/worker.js +++ b/packager/src/JSTransformer/worker/worker.js @@ -19,8 +19,9 @@ const invariant = require('fbjs/lib/invariant'); const minify = require('./minify'); import type {LogEntry} from '../../Logger/Types'; +import type {MappingsMap} from '../../lib/SourceMap'; import type {LocalPath} from '../../node-haste/lib/toLocalPath'; -import type {Ast, Plugins as BabelPlugins, SourceMap as MappingsMap} from 'babel-core'; +import type {Ast, Plugins as BabelPlugins} from 'babel-core'; export type TransformedCode = { code: string, @@ -32,6 +33,7 @@ export type TransformedCode = { export type Transformer = { transform: ({| filename: string, + localPath: string, options: ExtraOptions & TransformOptions, plugins?: BabelPlugins, src: string, @@ -71,17 +73,18 @@ export type Data = { transformFileEndLogEntry: LogEntry, }; -type Callback = ( +type Callback = ( error: ?Error, - data: ?Data, + data: ?T, ) => mixed; function transformCode( transformer: Transformer<*>, filename: string, + localPath: LocalPath, sourceCode: string, options: Options, - callback: Callback, + callback: Callback, ) { invariant( !options.minify || options.transform.generateSourceMaps, @@ -105,6 +108,7 @@ function transformCode( try { transformed = transformer.transform({ filename, + localPath, options: options.transform, src: sourceCode, }); @@ -158,21 +162,22 @@ function transformCode( exports.transformAndExtractDependencies = ( transform: string, filename: string, + localPath: LocalPath, sourceCode: string, options: Options, - callback: Callback, + callback: Callback, ) => { babelRegisterOnly([transform]); /* $FlowFixMe: impossible to type a dynamic require */ const transformModule: Transformer<*> = require(transform); - transformCode(transformModule, filename, sourceCode, options, callback); + transformCode(transformModule, filename, localPath, sourceCode, options, callback); }; exports.minify = ( filename: string, code: string, sourceMap: MappingsMap, - callback: (error: ?Error, result: mixed) => mixed, + callback: Callback<{code: string, map: MappingsMap}>, ) => { var result; try { diff --git a/packager/src/ModuleGraph/worker/__tests__/transform-module-test.js b/packager/src/ModuleGraph/worker/__tests__/transform-module-test.js index ef45c17389d..7c202be98c7 100644 --- a/packager/src/ModuleGraph/worker/__tests__/transform-module-test.js +++ b/packager/src/ModuleGraph/worker/__tests__/transform-module-test.js @@ -97,11 +97,13 @@ describe('transforming JS modules:', () => { transformModule(sourceCode, options(variants), () => { expect(transformer.transform).toBeCalledWith({ filename, + localPath: filename, options: {...defaults, ...variants.dev}, src: sourceCode, }); expect(transformer.transform).toBeCalledWith({ filename, + localPath: filename, options: {...defaults, ...variants.prod}, src: sourceCode, }); diff --git a/packager/src/ModuleGraph/worker/transform-module.js b/packager/src/ModuleGraph/worker/transform-module.js index 38e5a60073f..15b2fceb5e4 100644 --- a/packager/src/ModuleGraph/worker/transform-module.js +++ b/packager/src/ModuleGraph/worker/transform-module.js @@ -72,6 +72,7 @@ function transformModule( Object.keys(variants).forEach(name => { tasks[name] = asyncify(() => transformer.transform({ filename, + localPath: filename, options: {...defaultTransformOptions, ...variants[name]}, src: code, })