From 5071907b27f59a54b02f12a579b545205f073cd8 Mon Sep 17 00:00:00 2001 From: David Aurelio Date: Wed, 4 May 2016 09:32:30 -0700 Subject: [PATCH] Add verbose module names to dev builds Summary: This intends to make the devx story better after switching to numeric module IDs: - `require(')` works again for dev builds - In dev builds, Systrace will use the verbose names of modules for markers Reviewed By: bestander Differential Revision: D3253318 fb-gh-sync-id: 3425d5086ce28634653a6c8c7f5f11afa1614902 fbshipit-source-id: 3425d5086ce28634653a6c8c7f5f11afa1614902 --- packager/react-packager/src/Bundler/Bundle.js | 4 ++- packager/react-packager/src/Bundler/index.js | 2 +- .../src/Resolver/__tests__/Resolver-test.js | 32 ++++++++++++++++--- packager/react-packager/src/Resolver/index.js | 15 +++++---- .../src/Resolver/polyfills/require.js | 24 ++++++++++++-- 5 files changed, 63 insertions(+), 14 deletions(-) diff --git a/packager/react-packager/src/Bundler/Bundle.js b/packager/react-packager/src/Bundler/Bundle.js index 4a172d109..c3cdc8041 100644 --- a/packager/react-packager/src/Bundler/Bundle.js +++ b/packager/react-packager/src/Bundler/Bundle.js @@ -17,13 +17,14 @@ const crypto = require('crypto'); const SOURCEMAPPING_URL = '\n\/\/# sourceMappingURL='; class Bundle extends BundleBase { - constructor({sourceMapUrl, minify} = {}) { + constructor({sourceMapUrl, dev, minify} = {}) { super(); this._sourceMap = false; this._sourceMapUrl = sourceMapUrl; this._shouldCombineSourceMaps = false; this._numPrependedModules = 0; this._numRequireCalls = 0; + this._dev = dev; this._minify = minify; this._ramBundle = null; // cached RAM Bundle @@ -39,6 +40,7 @@ class Bundle extends BundleBase { map: moduleTransport.map, meta: moduleTransport.meta, minify: this._minify, + dev: this._dev, }).then(({code, map}) => { // If we get a map from the transformer we'll switch to a mode // were we're combining the source maps as opposed to diff --git a/packager/react-packager/src/Bundler/index.js b/packager/react-packager/src/Bundler/index.js index 2c02809ed..197b24f3f 100644 --- a/packager/react-packager/src/Bundler/index.js +++ b/packager/react-packager/src/Bundler/index.js @@ -169,7 +169,7 @@ class Bundler { const moduleSystemDeps = this._resolver.getModuleSystemDependencies({dev, unbundle}); return this._bundle({ - bundle: new Bundle({minify, sourceMapUrl: options.sourceMapUrl}), + bundle: new Bundle({dev, minify, sourceMapUrl: options.sourceMapUrl}), moduleSystemDeps, ...options, }); diff --git a/packager/react-packager/src/Resolver/__tests__/Resolver-test.js b/packager/react-packager/src/Resolver/__tests__/Resolver-test.js index 0f53e2a0e..3d89226d3 100644 --- a/packager/react-packager/src/Resolver/__tests__/Resolver-test.js +++ b/packager/react-packager/src/Resolver/__tests__/Resolver-test.js @@ -333,7 +333,8 @@ describe('Resolver', function() { module: module, name: 'test module', code, - meta: {dependencyOffsets} + meta: {dependencyOffsets}, + dev: false, }).then(({code: processedCode}) => { expect(processedCode).toEqual([ `__d(${getModuleId(module)} /* test module */, function(global, require, module, exports) {` + @@ -348,6 +349,28 @@ describe('Resolver', function() { }); }); + pit('should add module transport names as third argument to `__d`', () => { + const module = createModule('test module'); + const code = 'arbitrary(code)' + const resolutionResponse = new ResolutionResponseMock({ + dependencies: [module], + mainModuleId: 'test module', + }); + return depResolver.wrapModule({ + resolutionResponse, + code, + module, + name: 'test module', + dev: true, + }).then(({code: processedCode}) => + expect(processedCode).toEqual([ + `__d(${getModuleId(module)} /* test module */, function(global, require, module, exports) {` + + code, + '}, "test module");' + ].join('\n')) + ); + }); + pit('should pass through passed-in source maps', () => { const module = createModule('test module'); const resolutionResponse = new ResolutionResponseMock({ @@ -400,7 +423,7 @@ describe('Resolver', function() { pit('should prefix JSON files with `module.exports=`', () => { return depResolver - .wrapModule({resolutionResponse, module, name: id, code}) + .wrapModule({resolutionResponse, module, name: id, code, dev: false}) .then(({code: processedCode}) => expect(processedCode).toEqual([ `__d(${getModuleId(module)} /* ${id} */, function(global, require, module, exports) {`, @@ -420,7 +443,7 @@ describe('Resolver', function() { depResolver = new Resolver({ projectRoot: '/root', getModuleId, - minifyCode + minifyCode, }); module = createModule(id); module.path = '/arbitrary/path.js'; @@ -440,7 +463,8 @@ describe('Resolver', function() { name: id, code, map: sourceMap, - minify: true + minify: true, + dev: false, }).then(() => { expect(minifyCode).toBeCalledWith(module.path, wrappedCode, sourceMap); }); diff --git a/packager/react-packager/src/Resolver/index.js b/packager/react-packager/src/Resolver/index.js index 9fd7e4aa8..772c067b1 100644 --- a/packager/react-packager/src/Resolver/index.js +++ b/packager/react-packager/src/Resolver/index.js @@ -241,6 +241,7 @@ class Resolver { map, code, meta = {}, + dev = true, minify = false }) { if (module.isJSON()) { @@ -257,7 +258,7 @@ class Resolver { code, meta.dependencyOffsets ); - code = defineModuleCode(moduleId, code, name); + code = defineModuleCode(moduleId, code, name, dev); } @@ -275,13 +276,15 @@ class Resolver { } } -function defineModuleCode(moduleName, code, verboseName = '') { +function defineModuleCode(moduleName, code, verboseName = '', dev = true) { return [ - `__d(`, + '__d(', `${JSON.stringify(moduleName)} /* ${verboseName} */, `, - `function(global, require, module, exports) {`, - `${code}`, - '\n});', + 'function(global, require, module, exports) {', + code, + '\n}', + dev ? `, ${JSON.stringify(verboseName)}` : '', + ');', ].join(''); } diff --git a/packager/react-packager/src/Resolver/polyfills/require.js b/packager/react-packager/src/Resolver/polyfills/require.js index 045927211..a8c66673b 100644 --- a/packager/react-packager/src/Resolver/polyfills/require.js +++ b/packager/react-packager/src/Resolver/polyfills/require.js @@ -13,6 +13,9 @@ global.require = require; global.__d = define; const modules = Object.create(null); +if (__DEV__) { + var verboseNamesToModuleIds = Object.create(null); +} function define(moduleId, factory) { if (moduleId in modules) { @@ -26,8 +29,14 @@ function define(moduleId, factory) { isInitialized: false, exports: undefined, }; - if (__DEV__) { // HMR + if (__DEV__) { + // HMR modules[moduleId].hot = createHotReloadingObject(); + + // DEBUGGABLE MODULES NAMES + // avoid unnecessary parameter in prod + const verboseName = modules[moduleId].verboseName = arguments[2]; + verboseNamesToModuleIds[verboseName] = moduleId; } } @@ -62,6 +71,17 @@ function loadModuleImplementation(moduleId, module) { module = modules[moduleId]; } + if (__DEV__ && !module) { + // allow verbose module names to be passed as module ID + module = modules[verboseNamesToModuleIds[moduleId]]; + if (module) { + console.warn( + `Requiring module '${moduleId}' by name is only supported for ` + + 'debugging purposes and will break in production' + ); + } + } + if (!module) { throw unknownModuleError(moduleId); } @@ -87,7 +107,7 @@ function loadModuleImplementation(moduleId, module) { const {factory} = module; try { if (__DEV__) { - Systrace.beginEvent('JS_require_' + moduleId); + Systrace.beginEvent('JS_require_' + (module.verboseName || moduleId)); } const moduleObject = {exports};