From 830646529a41e41d9cd39b87e4531168482e5778 Mon Sep 17 00:00:00 2001 From: Amjad Masad Date: Fri, 1 May 2015 17:05:24 -0700 Subject: [PATCH 1/2] [react-packager] Combine source maps coming from transformer Summary: @public Fixes [#393](https://github.com/facebook/react-native/issues/393). Currently the transformer assumes line-preserving compilers and defaults to a super-fast source map generation process. However, we need to support compilers that aren't preserving lines. I had feared this wuold slow down the server but I came about a little known peace of the spec that defines an "indexed source map" just for the purpose of concating files: https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k/edit Test Plan: 1. runJestTests.sh 2. run server and click around example apps 3. add a custom transporter like babel 4. add a custom file and a debugger statement 5. debug in chrome and make sure it works redbox still works --- .../__tests__/Transformer-test.js | 4 +- react-packager/src/JSTransformer/index.js | 8 +- react-packager/src/Packager/Package.js | 112 ++++++++++++--- .../src/Packager/__tests__/Package-test.js | 132 ++++++++++++++++-- .../src/Packager/__tests__/Packager-test.js | 56 ++++---- react-packager/src/Packager/index.js | 37 ++--- react-packager/src/lib/ModuleTransport.js | 38 +++++ 7 files changed, 308 insertions(+), 79 deletions(-) create mode 100644 react-packager/src/lib/ModuleTransport.js diff --git a/react-packager/src/JSTransformer/__tests__/Transformer-test.js b/react-packager/src/JSTransformer/__tests__/Transformer-test.js index 72845a2e..eb307a02 100644 --- a/react-packager/src/JSTransformer/__tests__/Transformer-test.js +++ b/react-packager/src/JSTransformer/__tests__/Transformer-test.js @@ -11,6 +11,7 @@ jest .dontMock('worker-farm') .dontMock('os') + .dontMock('../../lib/ModuleTransport') .dontMock('../index'); var OPTIONS = { @@ -37,7 +38,7 @@ describe('Transformer', function() { pit('should loadFileAndTransform', function() { workers.mockImpl(function(data, callback) { - callback(null, { code: 'transformed' }); + callback(null, { code: 'transformed', map: 'sourceMap' }); }); require('fs').readFile.mockImpl(function(file, callback) { callback(null, 'content'); @@ -47,6 +48,7 @@ describe('Transformer', function() { .then(function(data) { expect(data).toEqual({ code: 'transformed', + map: 'sourceMap', sourcePath: 'file', sourceCode: 'content' }); diff --git a/react-packager/src/JSTransformer/index.js b/react-packager/src/JSTransformer/index.js index 962eb7fe..33e01703 100644 --- a/react-packager/src/JSTransformer/index.js +++ b/react-packager/src/JSTransformer/index.js @@ -14,6 +14,7 @@ var Cache = require('./Cache'); var workerFarm = require('worker-farm'); var declareOpts = require('../lib/declareOpts'); var util = require('util'); +var ModuleTransport = require('../lib/ModuleTransport'); var readFile = Promise.promisify(fs.readFile); @@ -100,11 +101,12 @@ Transformer.prototype.loadFileAndTransform = function(filePath) { throw formatError(res.error, filePath, sourceCode); } - return { + return new ModuleTransport({ code: res.code, + map: res.map, sourcePath: filePath, - sourceCode: sourceCode - }; + sourceCode: sourceCode, + }); } ); }); diff --git a/react-packager/src/Packager/Package.js b/react-packager/src/Packager/Package.js index 67e31e47..c621f747 100644 --- a/react-packager/src/Packager/Package.js +++ b/react-packager/src/Packager/Package.js @@ -11,6 +11,7 @@ var _ = require('underscore'); var base64VLQ = require('./base64-vlq'); var UglifyJS = require('uglify-js'); +var ModuleTransport = require('../lib/ModuleTransport'); module.exports = Package; @@ -19,22 +20,25 @@ function Package(sourceMapUrl) { this._modules = []; this._assets = []; this._sourceMapUrl = sourceMapUrl; + this._shouldCombineSourceMaps = false; } Package.prototype.setMainModuleId = function(moduleId) { this._mainModuleId = moduleId; }; -Package.prototype.addModule = function( - transformedCode, - sourceCode, - sourcePath -) { - this._modules.push({ - transformedCode: transformedCode, - sourceCode: sourceCode, - sourcePath: sourcePath - }); +Package.prototype.addModule = function(module) { + if (!(module instanceof ModuleTransport)) { + throw new Error('Expeceted a ModuleTransport object'); + } + + // If we get a map from the transformer we'll switch to a mode + // were we're combining the source maps as opposed to + if (!this._shouldCombineSourceMaps && module.map != null) { + this._shouldCombineSourceMaps = true; + } + + this._modules.push(module); }; Package.prototype.addAsset = function(asset) { @@ -45,11 +49,12 @@ Package.prototype.finalize = function(options) { options = options || {}; if (options.runMainModule) { var runCode = ';require("' + this._mainModuleId + '");'; - this.addModule( - runCode, - runCode, - 'RunMainModule.js' - ); + this.addModule(new ModuleTransport({ + code: runCode, + virtual: true, + sourceCode: runCode, + sourcePath: 'RunMainModule.js' + })); } Object.freeze(this._modules); @@ -67,7 +72,7 @@ Package.prototype._assertFinalized = function() { Package.prototype._getSource = function() { if (this._source == null) { - this._source = _.pluck(this._modules, 'transformedCode').join('\n'); + this._source = _.pluck(this._modules, 'code').join('\n'); } return this._source; }; @@ -136,10 +141,50 @@ Package.prototype.getMinifiedSourceAndMap = function() { } }; +/** + * I found a neat trick in the sourcemap spec that makes it easy + * to concat sourcemaps. The `sections` field allows us to combine + * the sourcemap easily by adding an offset. Tested on chrome. + * Seems like it's not yet in Firefox but that should be fine for + * now. + */ +Package.prototype._getCombinedSourceMaps = function(options) { + var result = { + version: 3, + file: 'bundle.js', + sections: [], + }; + + var line = 0; + this._modules.forEach(function(module) { + var map = module.map; + if (module.virtual) { + map = generateSourceMapForVirtualModule(module); + } + + if (options.excludeSource) { + map = _.extend({}, map, {sourcesContent: []}); + } + + result.sections.push({ + offset: { line: line, column: 0 }, + map: map, + }); + line += module.code.split('\n').length; + }); + + return result; +}; + Package.prototype.getSourceMap = function(options) { this._assertFinalized(); options = options || {}; + + if (this._shouldCombineSourceMaps) { + return this._getCombinedSourceMaps(options); + } + var mappings = this._getMappings(); var map = { file: 'bundle.js', @@ -168,13 +213,14 @@ Package.prototype._getMappings = function() { // except for the lineno mappinp: curLineno - prevLineno = 1; Which is C. var line = 'AACA'; + var moduleLines = Object.create(null); var mappings = ''; for (var i = 0; i < modules.length; i++) { var module = modules[i]; - var transformedCode = module.transformedCode; + var code = module.code; var lastCharNewLine = false; - module.lines = 0; - for (var t = 0; t < transformedCode.length; t++) { + moduleLines[module.sourcePath] = 0; + for (var t = 0; t < code.length; t++) { if (t === 0 && i === 0) { mappings += firstLine; } else if (t === 0) { @@ -183,13 +229,15 @@ Package.prototype._getMappings = function() { // This is the only place were we actually don't know the mapping ahead // of time. When it's a new module (and not the first) the lineno // mapping is 0 (current) - number of lines in prev module. - mappings += base64VLQ.encode(0 - modules[i - 1].lines); + mappings += base64VLQ.encode( + 0 - moduleLines[modules[i - 1].sourcePath] + ); mappings += 'A'; } else if (lastCharNewLine) { - module.lines++; + moduleLines[module.sourcePath]++; mappings += line; } - lastCharNewLine = transformedCode[t] === '\n'; + lastCharNewLine = code[t] === '\n'; if (lastCharNewLine) { mappings += ';'; } @@ -218,7 +266,25 @@ Package.prototype.getDebugInfo = function() { this._modules.map(function(m) { return '

Path:

' + m.sourcePath + '

Source:

' + '
'; + _.escape(m.code) + ''; }).join('\n'), ].join('\n'); }; + +function generateSourceMapForVirtualModule(module) { + // All lines map 1-to-1 + var mappings = 'AAAA;'; + + for (var i = 1; i < module.code.split('\n').length; i++) { + mappings += 'AACA;'; + } + + return { + version: 3, + sources: [ module.sourcePath ], + names: [], + mappings: mappings, + file: module.sourcePath, + sourcesContent: [ module.code ], + }; +} diff --git a/react-packager/src/Packager/__tests__/Package-test.js b/react-packager/src/Packager/__tests__/Package-test.js index db596a7b..0aaa3971 100644 --- a/react-packager/src/Packager/__tests__/Package-test.js +++ b/react-packager/src/Packager/__tests__/Package-test.js @@ -13,11 +13,13 @@ jest.autoMockOff(); var SourceMapGenerator = require('source-map').SourceMapGenerator; describe('Package', function() { + var ModuleTransport; var Package; var ppackage; beforeEach(function() { Package = require('../Package'); + ModuleTransport = require('../../lib/ModuleTransport'); ppackage = new Package('test_url'); ppackage.getSourceMap = jest.genMockFn().mockImpl(function() { return 'test-source-map'; @@ -26,8 +28,17 @@ describe('Package', function() { describe('source package', function() { it('should create a package and get the source', function() { - ppackage.addModule('transformed foo;', 'source foo', 'foo path'); - ppackage.addModule('transformed bar;', 'source bar', 'bar path'); + ppackage.addModule(new ModuleTransport({ + code: 'transformed foo;', + sourceCode: 'source foo', + sourcePath: 'foo path', + })); + ppackage.addModule(new ModuleTransport({ + code: 'transformed bar;', + sourceCode: 'source bar', + sourcePath: 'bar path', + })); + ppackage.finalize({}); expect(ppackage.getSource()).toBe([ 'transformed foo;', @@ -37,8 +48,18 @@ describe('Package', function() { }); it('should create a package and add run module code', function() { - ppackage.addModule('transformed foo;', 'source foo', 'foo path'); - ppackage.addModule('transformed bar;', 'source bar', 'bar path'); + ppackage.addModule(new ModuleTransport({ + code: 'transformed foo;', + sourceCode: 'source foo', + sourcePath: 'foo path' + })); + + ppackage.addModule(new ModuleTransport({ + code: 'transformed bar;', + sourceCode: 'source bar', + sourcePath: 'bar path' + })); + ppackage.setMainModuleId('foo'); ppackage.finalize({runMainModule: true}); expect(ppackage.getSource()).toBe([ @@ -59,7 +80,11 @@ describe('Package', function() { return minified; }; - ppackage.addModule('transformed foo;', 'source foo', 'foo path'); + ppackage.addModule(new ModuleTransport({ + code: 'transformed foo;', + sourceCode: 'source foo', + sourcePath: 'foo path' + })); ppackage.finalize(); expect(ppackage.getMinifiedSourceAndMap()).toBe(minified); }); @@ -68,13 +93,104 @@ describe('Package', function() { describe('sourcemap package', function() { it('should create sourcemap', function() { var p = new Package('test_url'); - p.addModule('transformed foo;\n', 'source foo', 'foo path'); - p.addModule('transformed bar;\n', 'source bar', 'bar path'); + p.addModule(new ModuleTransport({ + code: [ + 'transformed foo', + 'transformed foo', + 'transformed foo', + ].join('\n'), + sourceCode: [ + 'source foo', + 'source foo', + 'source foo', + ].join('\n'), + sourcePath: 'foo path', + })); + p.addModule(new ModuleTransport({ + code: [ + 'transformed bar', + 'transformed bar', + 'transformed bar', + ].join('\n'), + sourceCode: [ + 'source bar', + 'source bar', + 'source bar', + ].join('\n'), + sourcePath: 'bar path', + })); + p.setMainModuleId('foo'); p.finalize({runMainModule: true}); var s = p.getSourceMap(); expect(s).toEqual(genSourceMap(p._modules)); }); + + it('should combine sourcemaps', function() { + var p = new Package('test_url'); + + p.addModule(new ModuleTransport({ + code: 'transformed foo;\n', + map: {name: 'sourcemap foo'}, + sourceCode: 'source foo', + sourcePath: 'foo path' + })); + + p.addModule(new ModuleTransport({ + code: 'transformed foo;\n', + map: {name: 'sourcemap bar'}, + sourceCode: 'source foo', + sourcePath: 'foo path' + })); + + p.addModule(new ModuleTransport({ + code: 'image module;\nimage module;', + virtual: true, + sourceCode: 'image module;\nimage module;', + sourcePath: 'image.png', + })); + + p.setMainModuleId('foo'); + p.finalize({runMainModule: true}); + + var s = p.getSourceMap(); + expect(s).toEqual({ + file: 'bundle.js', + version: 3, + sections: [ + { offset: { line: 0, column: 0 }, map: { name: 'sourcemap foo' } }, + { offset: { line: 2, column: 0 }, map: { name: 'sourcemap bar' } }, + { + offset: { + column: 0, + line: 4 + }, + map: { + file: 'image.png', + mappings: 'AAAA;AACA;', + names: {}, + sources: [ 'image.png' ], + sourcesContent: ['image module;\nimage module;'], + version: 3, + } + }, + { + offset: { + column: 0, + line: 6 + }, + map: { + file: 'RunMainModule.js', + mappings: 'AAAA;', + names: {}, + sources: [ 'RunMainModule.js' ], + sourcesContent: [';require("foo");'], + version: 3, + } + } + ], + }); + }); }); describe('getAssets()', function() { @@ -95,7 +211,7 @@ describe('Package', function() { var packageLineNo = 0; for (var i = 0; i < modules.length; i++) { var module = modules[i]; - var transformedCode = module.transformedCode; + var transformedCode = module.code; var sourcePath = module.sourcePath; var sourceCode = module.sourceCode; var transformedLineCount = 0; diff --git a/react-packager/src/Packager/__tests__/Packager-test.js b/react-packager/src/Packager/__tests__/Packager-test.js index 8e1420a3..3f093446 100644 --- a/react-packager/src/Packager/__tests__/Packager-test.js +++ b/react-packager/src/Packager/__tests__/Packager-test.js @@ -13,6 +13,7 @@ jest .dontMock('path') .dontMock('os') .dontMock('underscore') + .dontMock('../../lib/ModuleTransport') .setMock('uglify-js') .dontMock('../'); @@ -93,6 +94,7 @@ describe('Packager', function() { .mockImpl(function(path) { return Promise.resolve({ code: 'transformed ' + path, + map: 'sourcemap ' + path, sourceCode: 'source ' + path, sourcePath: path }); @@ -117,16 +119,19 @@ describe('Packager', function() { return packager.package('/root/foo.js', true, 'source_map_url') .then(function(p) { - expect(p.addModule.mock.calls[0]).toEqual([ - 'lol transformed /root/foo.js lol', - 'source /root/foo.js', - '/root/foo.js' - ]); - expect(p.addModule.mock.calls[1]).toEqual([ - 'lol transformed /root/bar.js lol', - 'source /root/bar.js', - '/root/bar.js' - ]); + expect(p.addModule.mock.calls[0][0]).toEqual({ + code: 'lol transformed /root/foo.js lol', + map: 'sourcemap /root/foo.js', + sourceCode: 'source /root/foo.js', + sourcePath: '/root/foo.js', + }); + + expect(p.addModule.mock.calls[1][0]).toEqual({ + code: 'lol transformed /root/bar.js lol', + map: 'sourcemap /root/bar.js', + sourceCode: 'source /root/bar.js', + sourcePath: '/root/bar.js' + }); var imgModule_DEPRECATED = { __packager_asset: true, @@ -138,15 +143,15 @@ describe('Packager', function() { deprecated: true, }; - expect(p.addModule.mock.calls[2]).toEqual([ - 'lol module.exports = ' + + expect(p.addModule.mock.calls[2][0]).toEqual({ + code: 'lol module.exports = ' + JSON.stringify(imgModule_DEPRECATED) + '; lol', - 'module.exports = ' + + sourceCode: 'module.exports = ' + JSON.stringify(imgModule_DEPRECATED) + ';', - '/root/img/img.png' - ]); + sourcePath: '/root/img/img.png' + }); var imgModule = { __packager_asset: true, @@ -160,21 +165,21 @@ describe('Packager', function() { type: 'png', }; - expect(p.addModule.mock.calls[3]).toEqual([ - 'lol module.exports = ' + + expect(p.addModule.mock.calls[3][0]).toEqual({ + code: 'lol module.exports = ' + JSON.stringify(imgModule) + '; lol', - 'module.exports = ' + + sourceCode: 'module.exports = ' + JSON.stringify(imgModule) + ';', - '/root/img/new_image.png' - ]); + sourcePath: '/root/img/new_image.png' + }); - expect(p.addModule.mock.calls[4]).toEqual([ - 'lol module.exports = {"json":true}; lol', - 'module.exports = {"json":true};', - '/root/file.json' - ]); + expect(p.addModule.mock.calls[4][0]).toEqual({ + code: 'lol module.exports = {"json":true}; lol', + sourceCode: 'module.exports = {"json":true};', + sourcePath: '/root/file.json' + }); expect(p.finalize.mock.calls[0]).toEqual([ {runMainModule: true} @@ -189,5 +194,4 @@ describe('Packager', function() { ]); }); }); - }); diff --git a/react-packager/src/Packager/index.js b/react-packager/src/Packager/index.js index 8563e272..c03a0432 100644 --- a/react-packager/src/Packager/index.js +++ b/react-packager/src/Packager/index.js @@ -14,9 +14,9 @@ var path = require('path'); var Promise = require('bluebird'); var Transformer = require('../JSTransformer'); var DependencyResolver = require('../DependencyResolver'); -var _ = require('underscore'); var Package = require('./Package'); var Activity = require('../Activity'); +var ModuleTransport = require('../lib/ModuleTransport'); var declareOpts = require('../lib/declareOpts'); var imageSize = require('image-size'); @@ -125,12 +125,8 @@ Packager.prototype.package = function(main, runModule, sourceMapUrl, isDev) { .then(function(transformedModules) { Activity.endEvent(transformEventId); - transformedModules.forEach(function(transformed) { - ppackage.addModule( - transformed.code, - transformed.sourceCode, - transformed.sourcePath - ); + transformedModules.forEach(function(moduleTransport) { + ppackage.addModule(moduleTransport); }); ppackage.finalize({ runMainModule: runModule }); @@ -163,11 +159,13 @@ Packager.prototype._transformModule = function(ppackage, module) { var resolver = this._resolver; return transform.then(function(transformed) { - return _.extend( - {}, - transformed, - {code: resolver.wrapModule(module, transformed.code)} - ); + var code = resolver.wrapModule(module, transformed.code); + return new ModuleTransport({ + code: code, + map: transformed.map, + sourceCode: transformed.sourceCode, + sourcePath: transformed.sourcePath, + }); }); }; @@ -191,11 +189,12 @@ Packager.prototype.generateAssetModule_DEPRECATED = function(ppackage, module) { var code = 'module.exports = ' + JSON.stringify(img) + ';'; - return { + return new ModuleTransport({ code: code, sourceCode: code, sourcePath: module.path, - }; + virtual: true, + }); }); }; @@ -222,11 +221,12 @@ Packager.prototype.generateAssetModule = function(ppackage, module) { var code = 'module.exports = ' + JSON.stringify(img) + ';'; - return { + return new ModuleTransport({ code: code, sourceCode: code, sourcePath: module.path, - }; + virtual: true, + }); }); }; @@ -234,11 +234,12 @@ function generateJSONModule(module) { return readFile(module.path).then(function(data) { var code = 'module.exports = ' + data.toString('utf8') + ';'; - return { + return new ModuleTransport({ code: code, sourceCode: code, sourcePath: module.path, - }; + virtual: true, + }); }); } diff --git a/react-packager/src/lib/ModuleTransport.js b/react-packager/src/lib/ModuleTransport.js new file mode 100644 index 00000000..a5f1d568 --- /dev/null +++ b/react-packager/src/lib/ModuleTransport.js @@ -0,0 +1,38 @@ +/** + * Copyright (c) 2015-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ +'use strict'; + +function ModuleTransport(data) { + assertExists(data, 'code'); + this.code = data.code; + + assertExists(data, 'sourceCode'); + this.sourceCode = data.sourceCode; + + assertExists(data, 'sourcePath'); + this.sourcePath = data.sourcePath; + + this.virtual = data.virtual; + + if (this.virtual && data.map) { + throw new Error('Virtual modules cannot have source maps'); + } + + this.map = data.map; + + Object.freeze(this); +} + +module.exports = ModuleTransport; + +function assertExists(obj, field) { + if (obj[field] == null) { + throw new Error('Modules must have `' + field + '`'); + } +} From 95a120b66319062a27606a0655aadfc2f68f1c6f Mon Sep 17 00:00:00 2001 From: Spencer Ahrens Date: Mon, 4 May 2015 18:34:29 -0700 Subject: [PATCH 2/2] [ReactNative] improve console logging a little bit --- .../haste/polyfills/console.js | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/react-packager/src/DependencyResolver/haste/polyfills/console.js b/react-packager/src/DependencyResolver/haste/polyfills/console.js index 91fb970f..57576961 100644 --- a/react-packager/src/DependencyResolver/haste/polyfills/console.js +++ b/react-packager/src/DependencyResolver/haste/polyfills/console.js @@ -35,25 +35,34 @@ function getNativeLogFunction(level) { return function() { var str = Array.prototype.map.call(arguments, function(arg) { - if (arg == null) { - return arg === null ? 'null' : 'undefined'; - } else if (typeof arg === 'string') { - return '"' + arg + '"'; + var ret; + var type = typeof arg; + if (arg === null) { + ret = 'null'; + } else if (arg === undefined) { + ret = 'undefined'; + } else if (type === 'string') { + ret = '"' + arg + '"'; + } else if (type === 'function') { + try { + ret = arg.toString(); + } catch (e) { + ret = '[function unknown]'; + } } else { // Perform a try catch, just in case the object has a circular // reference or stringify throws for some other reason. try { - return JSON.stringify(arg); + ret = JSON.stringify(arg); } catch (e) { if (typeof arg.toString === 'function') { try { - return arg.toString(); - } catch (E) { - return 'unknown'; - } + ret = arg.toString(); + } catch (E) {} } } } + return ret || '["' + type + '" failed to stringify]'; }).join(', '); global.nativeLoggingHook(str, level); };