From 1f2e16c0afee51e36e1eaccb384ee45253940ec2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Bigio?= Date: Wed, 27 Jan 2016 14:55:02 -0800 Subject: [PATCH] Hot Loading Sourcemaps Summary: public To make sourcemaps work on Hot Loading work, we'll need to be able to serve them for each module that is dynamically replaced. To do so we introduced a new parameter to the bundler, namely `entryModuleOnly` to decide whether or not to process the full dependency tree or just the module associated to the entry file. Also we need to add `//sourceMappingURL` to the HMR updates so that in case of an error the runtime retrieves the sourcemaps for the file on which an error occurred from the server. Finally, we need to refactor a bit how we load the HMR updates into JSC. Unfortunately, if the code is eval'ed when an error is thrown, the line and column number are missing. This is a bug/missing feature in JSC. To walkaround the issue we need to eval the code on native. This adds a bit of complexity to HMR as for both platforms we'll have to have a thin module to inject code but I don't see any other alternative. when debugging this is not needed as Chrome supports sourceMappingURLs on eval'ed code Reviewed By: javache Differential Revision: D2841788 fb-gh-sync-id: ad9370d26894527a151cea722463e694c670227e --- react-packager/src/Bundler/Bundle.js | 10 ++- react-packager/src/Bundler/BundleBase.js | 4 + react-packager/src/Bundler/HMRBundle.js | 31 +++++--- .../src/Bundler/__tests__/Bundle-test.js | 4 +- react-packager/src/Bundler/index.js | 79 ++++++++++++++++--- .../src/Server/__tests__/Server-test.js | 4 + react-packager/src/Server/index.js | 9 +++ 7 files changed, 117 insertions(+), 24 deletions(-) diff --git a/react-packager/src/Bundler/Bundle.js b/react-packager/src/Bundler/Bundle.js index 2641550a..9700af08 100644 --- a/react-packager/src/Bundler/Bundle.js +++ b/react-packager/src/Bundler/Bundle.js @@ -206,7 +206,7 @@ class Bundle extends BundleBase { _getCombinedSourceMaps(options) { const result = { version: 3, - file: 'bundle.js', + file: this._getSourceMapFile(), sections: [], }; @@ -246,7 +246,7 @@ class Bundle extends BundleBase { const mappings = this._getMappings(); const map = { - file: 'bundle.js', + file: this._getSourceMapFile(), sources: _.pluck(super.getModules(), 'sourcePath'), version: 3, names: [], @@ -262,6 +262,12 @@ class Bundle extends BundleBase { return eTag; } + _getSourceMapFile() { + return this._sourceMapUrl + ? this._sourceMapUrl.replace('.map', '.bundle') + : 'bundle.js'; + } + _getMappings() { const modules = super.getModules(); diff --git a/react-packager/src/Bundler/BundleBase.js b/react-packager/src/Bundler/BundleBase.js index b9e1cbe3..79cbfefc 100644 --- a/react-packager/src/Bundler/BundleBase.js +++ b/react-packager/src/Bundler/BundleBase.js @@ -19,6 +19,10 @@ class BundleBase { this._mainModuleId = this._mainModuleName = undefined; } + isEmpty() { + return this._modules.length === 0 && this._assets.length === 0; + } + getMainModuleId() { return this._mainModuleId; } diff --git a/react-packager/src/Bundler/HMRBundle.js b/react-packager/src/Bundler/HMRBundle.js index fa8c95db..49a21dfd 100644 --- a/react-packager/src/Bundler/HMRBundle.js +++ b/react-packager/src/Bundler/HMRBundle.js @@ -8,12 +8,17 @@ */ 'use strict'; +const _ = require('underscore'); const BundleBase = require('./BundleBase'); const ModuleTransport = require('../lib/ModuleTransport'); class HMRBundle extends BundleBase { - constructor() { + constructor({sourceURLFn, sourceMappingURLFn}) { super(); + this._sourceURLFn = sourceURLFn + this._sourceMappingURLFn = sourceMappingURLFn; + this._sourceURLs = []; + this._sourceMappingURLs = []; } addModule(resolver, response, module, transformed) { @@ -21,14 +26,8 @@ class HMRBundle extends BundleBase { module, transformed.code, ).then(({name, code}) => { - code = ` - __accept( - '${name}', - function(global, require, module, exports) { - ${code} - } - ); - `; + // need to be in single line so that lines match on sourcemaps + code = `__accept(${JSON.stringify(name)}, function(global, require, module, exports) { ${code} });`; const moduleTransport = new ModuleTransport({ code, @@ -40,8 +39,22 @@ class HMRBundle extends BundleBase { }); super.addModule(moduleTransport); + this._sourceMappingURLs.push(this._sourceMappingURLFn(moduleTransport.sourcePath)); + this._sourceURLs.push(this._sourceURLFn(moduleTransport.sourcePath)); }); } + + getModulesCode() { + return this._modules.map(module => module.code); + } + + getSourceURLs() { + return this._sourceURLs; + } + + getSourceMappingURLs() { + return this._sourceMappingURLs; + } } module.exports = HMRBundle; diff --git a/react-packager/src/Bundler/__tests__/Bundle-test.js b/react-packager/src/Bundler/__tests__/Bundle-test.js index 5e5fca22..b4152a7b 100644 --- a/react-packager/src/Bundler/__tests__/Bundle-test.js +++ b/react-packager/src/Bundler/__tests__/Bundle-test.js @@ -214,7 +214,7 @@ describe('Bundle', () => { const sourceMap = otherBundle.getSourceMap({dev: true}); expect(sourceMap).toEqual({ - file: 'bundle.js', + file: 'test_url', version: 3, sections: [ { offset: { line: 0, column: 0 }, map: { name: 'sourcemap foo' } }, @@ -340,7 +340,7 @@ describe('Bundle', () => { function genSourceMap(modules) { - var sourceMapGen = new SourceMapGenerator({file: 'bundle.js', version: 3}); + var sourceMapGen = new SourceMapGenerator({file: 'test_url', version: 3}); var bundleLineNo = 0; for (var i = 0; i < modules.length; i++) { var module = modules[i]; diff --git a/react-packager/src/Bundler/index.js b/react-packager/src/Bundler/index.js index b6ff9133..d2852d44 100644 --- a/react-packager/src/Bundler/index.js +++ b/react-packager/src/Bundler/index.js @@ -166,9 +166,58 @@ class Bundler { }); } + _sourceHMRURL(platform, path) { + return this._hmrURL( + 'http://localhost:8081', // TODO: (martinb) avoid hardcoding + platform, + 'bundle', + path, + ); + } + + _sourceMappingHMRURL(platform, path) { + // Chrome expects `sourceURL` when eval'ing code + return this._hmrURL( + '\/\/# sourceURL=', + platform, + 'map', + path, + ); + } + + _hmrURL(prefix, platform, extensionOverride, path) { + const matchingRoot = this._projectRoots.find(root => path.startsWith(root)); + + if (!matchingRoot) { + throw new Error('No matching project root for ', path); + } + + const extensionStart = path.lastIndexOf('.'); + let resource = path.substring( + matchingRoot.length, + extensionStart !== -1 ? extensionStart : undefined, + ); + + const extension = extensionStart !== -1 + ? path.substring(extensionStart + 1) + : null; + + return ( + prefix + resource + + '.' + extensionOverride + '?' + + 'platform=' + platform + '&runModule=false&entryModuleOnly=true&hot=true' + ); + } + bundleForHMR(options) { return this._bundle({ - bundle: new HMRBundle(), + bundle: new HMRBundle({ + sourceURLFn: this._sourceHMRURL.bind(this, options.platform), + sourceMappingURLFn: this._sourceMappingHMRURL.bind( + this, + options.platform, + ), + }), hot: true, ...options, }); @@ -185,6 +234,7 @@ class Bundler { platform, unbundle: isUnbundle, hot: hot, + entryModuleOnly, }) { const findEventId = Activity.startEvent('find dependencies'); let transformEventId; @@ -195,18 +245,25 @@ class Bundler { bundle.setMainModuleName(response.mainModuleId); transformEventId = Activity.startEvent('transform'); - const moduleSystemDeps = includeSystemDependencies - ? this._resolver.getModuleSystemDependencies( - { dev: isDev, platform, isUnbundle } - ) - : []; + let dependencies; + if (entryModuleOnly) { + dependencies = response.dependencies.filter(module => + module.path.endsWith(entryFile) + ); + } else { + const moduleSystemDeps = includeSystemDependencies + ? this._resolver.getModuleSystemDependencies( + { dev: isDev, platform, isUnbundle } + ) + : []; - const modulesToProcess = modules || response.dependencies; - const dependencies = moduleSystemDeps.concat(modulesToProcess); + const modulesToProcess = modules || response.dependencies; + const dependencies = moduleSystemDeps.concat(modulesToProcess); - bundle.setNumPrependedModules && bundle.setNumPrependedModules( - response.numPrependedDependencies + moduleSystemDeps.length - ); + bundle.setNumPrependedModules && bundle.setNumPrependedModules( + response.numPrependedDependencies + moduleSystemDeps.length + ); + } let bar; if (process.stdout.isTTY) { diff --git a/react-packager/src/Server/__tests__/Server-test.js b/react-packager/src/Server/__tests__/Server-test.js index 123134f8..668e162f 100644 --- a/react-packager/src/Server/__tests__/Server-test.js +++ b/react-packager/src/Server/__tests__/Server-test.js @@ -144,6 +144,7 @@ describe('processRequest', () => { platform: undefined, runBeforeMainModule: ['InitializeJavaScriptAppEngine'], unbundle: false, + entryModuleOnly: false, }); }); }); @@ -165,6 +166,7 @@ describe('processRequest', () => { platform: 'ios', runBeforeMainModule: ['InitializeJavaScriptAppEngine'], unbundle: false, + entryModuleOnly: false, }); }); }); @@ -321,6 +323,7 @@ describe('processRequest', () => { platform: undefined, runBeforeMainModule: ['InitializeJavaScriptAppEngine'], unbundle: false, + entryModuleOnly: false, }) ); }); @@ -341,6 +344,7 @@ describe('processRequest', () => { platform: undefined, runBeforeMainModule: ['InitializeJavaScriptAppEngine'], unbundle: false, + entryModuleOnly: false, }) ); }); diff --git a/react-packager/src/Server/index.js b/react-packager/src/Server/index.js index 99a3472d..559fb302 100644 --- a/react-packager/src/Server/index.js +++ b/react-packager/src/Server/index.js @@ -118,6 +118,10 @@ const bundleOpts = declareOpts({ type: 'boolean', default: false, }, + entryModuleOnly: { + type: 'boolean', + default: false, + }, }); const dependencyOpts = declareOpts({ @@ -527,6 +531,11 @@ class Server { false ), platform: platform, + entryModuleOnly: this._getBoolOptionFromQuery( + urlObj.query, + 'entryModuleOnly', + false, + ), }; }