From ee637ee0eb597e0a5caf7ec6073c87afb8bb1827 Mon Sep 17 00:00:00 2001 From: Jean Lauliac Date: Thu, 10 Nov 2016 09:06:59 -0800 Subject: [PATCH] packager worker/index.js: do not write to cache directly Reviewed By: davidaurelio Differential Revision: D4159970 fbshipit-source-id: 133ce5797e94a39fe60cbb9983edd561ce7d818e --- react-packager/src/Bundler/index.js | 1 - .../__tests__/Transformer-test.js | 8 +- react-packager/src/JSTransformer/index.js | 4 +- .../worker/__tests__/worker-test.js | 83 ++++++++----------- .../src/JSTransformer/worker/index.js | 18 +--- react-packager/src/node-haste/Module.js | 14 +++- 6 files changed, 52 insertions(+), 76 deletions(-) diff --git a/react-packager/src/Bundler/index.js b/react-packager/src/Bundler/index.js index 1acd82d8..613bafd5 100644 --- a/react-packager/src/Bundler/index.js +++ b/react-packager/src/Bundler/index.js @@ -202,7 +202,6 @@ class Bundler { module.path, code, transformCodeOptions, - transformCacheKey, ), transformCacheKey, }); diff --git a/react-packager/src/JSTransformer/__tests__/Transformer-test.js b/react-packager/src/JSTransformer/__tests__/Transformer-test.js index 4689d7fb..d99e1360 100644 --- a/react-packager/src/JSTransformer/__tests__/Transformer-test.js +++ b/react-packager/src/JSTransformer/__tests__/Transformer-test.js @@ -27,7 +27,6 @@ const {any} = jasmine; describe('Transformer', function() { let options, workers, Cache; const fileName = '/an/arbitrary/file.js'; - const transformCacheKey = 'abcdef'; const transformModulePath = __filename; beforeEach(function() { @@ -48,13 +47,12 @@ describe('Transformer', function() { ' and options to the worker farm when transforming', () => { const transformOptions = {arbitrary: 'options'}; const code = 'arbitrary(code)'; - new Transformer(options).transformFile(fileName, code, transformOptions, transformCacheKey); + new Transformer(options).transformFile(fileName, code, transformOptions); expect(workers.transformAndExtractDependencies).toBeCalledWith( transformModulePath, fileName, code, transformOptions, - transformCacheKey, any(Function), ); }); @@ -65,7 +63,7 @@ describe('Transformer', function() { var snippet = 'snippet'; workers.transformAndExtractDependencies.mockImpl( - function(transformPath, filename, code, opts, transfCacheKey, callback) { + function(transformPath, filename, code, opts, callback) { var babelError = new SyntaxError(message); babelError.type = 'SyntaxError'; babelError.description = message; @@ -78,7 +76,7 @@ describe('Transformer', function() { }, ); - return transformer.transformFile(fileName, '', {}, transformCacheKey) + return transformer.transformFile(fileName, '', {}) .catch(function(error) { expect(error.type).toEqual('TransformError'); expect(error.message).toBe('SyntaxError ' + message); diff --git a/react-packager/src/JSTransformer/index.js b/react-packager/src/JSTransformer/index.js index 54e2d8e0..92c26056 100644 --- a/react-packager/src/JSTransformer/index.js +++ b/react-packager/src/JSTransformer/index.js @@ -108,13 +108,13 @@ class Transformer { this._workers && workerFarm.end(this._workers); } - transformFile(fileName, code, options, transformCacheKey) { + transformFile(fileName, code, options) { if (!this._transform) { return Promise.reject(new Error('No transform module')); } debug('transforming file', fileName); return this - ._transform(this._transformModulePath, fileName, code, options, transformCacheKey) + ._transform(this._transformModulePath, fileName, code, options) .then(data => { Logger.log(data.transformFileStartLogEntry); Logger.log(data.transformFileEndLogEntry); diff --git a/react-packager/src/JSTransformer/worker/__tests__/worker-test.js b/react-packager/src/JSTransformer/worker/__tests__/worker-test.js index d901921c..37636f8d 100644 --- a/react-packager/src/JSTransformer/worker/__tests__/worker-test.js +++ b/react-packager/src/JSTransformer/worker/__tests__/worker-test.js @@ -13,13 +13,11 @@ jest.mock('../constant-folding'); jest.mock('../extract-dependencies'); jest.mock('../inline'); jest.mock('../minify'); -jest.mock('../../../lib/TransformCache'); const {any, objectContaining} = jasmine; describe('code transformation worker:', () => { let transformCode; - let TransformCache; let extractDependencies, transform; beforeEach(() => { @@ -28,14 +26,13 @@ describe('code transformation worker:', () => { extractDependencies = require('../extract-dependencies').mockReturnValue({}); transform = jest.fn(); - TransformCache = require('../../../lib/TransformCache'); }); 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}, ''); + transformCode(transform, filename, sourceCode, {transform: transformOptions}); expect(transform).toBeCalledWith( {filename, sourceCode, options: transformOptions}, any(Function)); }); @@ -43,7 +40,7 @@ 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 sourceCode = '{"arbitrary":"property"}'; - transformCode(transform, filename, sourceCode, {}, ''); + transformCode(transform, filename, sourceCode, {}); expect(transform).toBeCalledWith( {filename, sourceCode: `module.exports=${sourceCode}`}, any(Function)); }); @@ -56,10 +53,9 @@ describe('code transformation worker:', () => { transform.mockImplementation((_, callback) => callback(null, result)); - transformCode(transform, 'filename', 'code', {}, '', error => { + transformCode(transform, 'filename', 'code', {}, (error, data) => { expect(error).toBeNull(); - expect(TransformCache.mock.lastWrite.result) - .toEqual(objectContaining(result)); + expect(data.result).toEqual(objectContaining(result)); done(); }); }); @@ -71,14 +67,14 @@ describe('code transformation worker:', () => { const result = { code: 'p.exports={a:1,b:2}', }; - transform.mockImplementation((_, callback) => - callback(null, result)); - - transformCode(transform, 'arbitrary/file.json', 'b', {}, '', error => { - expect(error).toBeNull(); - expect(TransformCache.mock.lastWrite.result.code).toEqual('{a:1,b:2}'); - done(); - }); + transform.mockImplementation((_, callback) => callback(null, result)); + const filePath = 'arbitrary/file.json'; + transformCode(transform, filePath, 'b', {}, (error, data) => { + expect(error).toBeNull(); + expect(data.result.code).toEqual('{a:1,b:2}'); + done(); + }, + ); } ); @@ -88,9 +84,10 @@ describe('code transformation worker:', () => { code: `${shebang} \n arbitrary(code)`, }; transform.mockImplementation((_, callback) => callback(null, result)); - transformCode(transform, 'arbitrary/file.js', 'b', {}, '', error => { + const filePath = 'arbitrary/file.js'; + transformCode(transform, filePath, 'b', {}, (error, data) => { expect(error).toBeNull(); - const code = TransformCache.mock.lastWrite.result.code; + const {code} = data.result; expect(code).not.toContain(shebang); expect(code.split('\n').length).toEqual(result.code.split('\n').length); done(); @@ -100,7 +97,7 @@ 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 => { + transformCode(transform, 'filename', 'code', {}, e => { expect(e).toBe(error); done(); }); @@ -117,7 +114,7 @@ describe('code transformation worker:', () => { it('passes the transformed code the `extractDependencies`', done => { code = 'arbitrary(code)'; - transformCode(transform, 'filename', 'code', {}, '', (error) => { + transformCode(transform, 'filename', 'code', {}, (error) => { expect(error).toBeNull(); expect(extractDependencies).toBeCalledWith(code); done(); @@ -134,45 +131,31 @@ describe('code transformation worker:', () => { }; extractDependencies.mockReturnValue(dependencyData); - transformCode(transform, 'filename', 'code', {}, '', error => { + transformCode(transform, 'filename', 'code', {}, (error, data) => { expect(error).toBeNull(); - const data = TransformCache.mock.lastWrite.result; - expect(data).toEqual(objectContaining(dependencyData)); + expect(data.result).toEqual(objectContaining(dependencyData)); done(); }); } ); it('does not extract requires if files are marked as "extern"', done => { - transformCode( - transform, - 'filename', - 'code', - {extern: true}, - '', - error => { + const opts = {extern: true}; + transformCode(transform, 'filename', 'code', opts, (error, data) => { expect(error).toBeNull(); - const {dependencies, dependencyOffsets} = - TransformCache.mock.lastWrite.result; + const {dependencies, dependencyOffsets} = data.result; expect(extractDependencies).not.toBeCalled(); expect(dependencies).toEqual([]); expect(dependencyOffsets).toEqual([]); done(); - } - ); + }); }); it('does not extract requires of JSON files', done => { - transformCode( - transform, - 'arbitrary.json', - '{"arbitrary":"json"}', - {}, - '', - error => { + const jsonStr = '{"arbitrary":"json"}'; + transformCode(transform, 'arbitrary.json', jsonStr, {}, (error, data) => { expect(error).toBeNull(); - const {dependencies, dependencyOffsets} = - TransformCache.mock.lastWrite.result; + const {dependencies, dependencyOffsets} = data.result; expect(extractDependencies).not.toBeCalled(); expect(dependencies).toEqual([]); expect(dependencyOffsets).toEqual([]); @@ -210,7 +193,7 @@ describe('code transformation worker:', () => { it('passes the transform result to `inline` for constant inlining', done => { transformResult = {map: {version: 3}, code: 'arbitrary(code)'}; - transformCode(transform, filename, 'code', options, '', () => { + transformCode(transform, filename, 'code', options, () => { expect(inline).toBeCalledWith(filename, transformResult, options); done(); }); @@ -219,30 +202,30 @@ 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(transform, 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(transform, filename, 'code', options, () => { expect(extractDependencies).toBeCalledWith(foldedCode); done(); }); }); it('uses the dependencies obtained from the optimized result', done => { - transformCode(transform, filename, 'code', options, '', () => { - const result = TransformCache.mock.lastWrite.result; + transformCode(transform, filename, 'code', options, (_, data) => { + const result = data.result; expect(result.dependencies).toEqual(dependencyData.dependencies); done(); }); }); it('uses data produced by `constant-folding` for the result', done => { - transformCode(transform, 'filename', 'code', options, '', () => { - expect(TransformCache.mock.lastWrite.result) + transformCode(transform, 'filename', 'code', options, (_, data) => { + expect(data.result) .toEqual(objectContaining({code: foldedCode, map: foldedMap})); done(); }); diff --git a/react-packager/src/JSTransformer/worker/index.js b/react-packager/src/JSTransformer/worker/index.js index e6425328..74d862cd 100644 --- a/react-packager/src/JSTransformer/worker/index.js +++ b/react-packager/src/JSTransformer/worker/index.js @@ -8,12 +8,6 @@ */ 'use strict'; -require('../../../../babelRegisterOnly')([ - /packager\/react-packager\/src\/lib\/TransformCache/, -]); - -const TransformCache = require('../../lib/TransformCache'); - const constantFolding = require('./constant-folding'); const extractDependencies = require('./extract-dependencies'); const inline = require('./inline'); @@ -26,7 +20,7 @@ function makeTransformParams(filename, sourceCode, options) { return {filename, sourceCode, options}; } -function transformCode(transform, filename, sourceCode, options, transformCacheKey, callback) { +function transformCode(transform, filename, sourceCode, options, callback) { const params = makeTransformParams(filename, sourceCode, options.transform); const isJson = filename.endsWith('.json'); @@ -79,13 +73,6 @@ function transformCode(transform, filename, sourceCode, options, transformCacheK result.code = code; result.map = map; - TransformCache.writeSync({ - filePath: filename, - sourceCode, - transformCacheKey, - transformOptions: options, - result, - }); return callback(null, { result, transformFileStartLogEntry, @@ -99,10 +86,9 @@ exports.transformAndExtractDependencies = ( filename, sourceCode, options, - transformCacheKey, callback ) => { - transformCode(require(transform), filename, sourceCode, options || {}, transformCacheKey, callback); + transformCode(require(transform), filename, sourceCode, options || {}, callback); }; exports.minify = (filename, code, sourceMap, callback) => { diff --git a/react-packager/src/node-haste/Module.js b/react-packager/src/node-haste/Module.js index d3364bf9..b89b1453 100644 --- a/react-packager/src/node-haste/Module.js +++ b/react-packager/src/node-haste/Module.js @@ -225,13 +225,23 @@ class Module { transformOptions: mixed, callback: (error: ?Error, result: ?TransformedCode) => void, ) { - const {_transformCode} = this; + const {_transformCode, _transformCacheKey} = this; // AssetModule_DEPRECATED doesn't provide transformCode, but these should // never be transformed anyway. invariant(_transformCode != null, 'missing code transform funtion'); + invariant(_transformCacheKey != null, 'missing cache key'); this._readSourceCode().then(sourceCode => { return _transformCode(this, sourceCode, transformOptions) - .then(freshResult => callback(undefined, freshResult)); + .then(freshResult => { + TransformCache.writeSync({ + filePath: this.path, + sourceCode, + transformCacheKey: _transformCacheKey, + transformOptions, + result: freshResult, + }); + callback(undefined, freshResult); + }); }, callback); }