From 03a85ea8c2c045fb4d5e322fe84747f30971dcc4 Mon Sep 17 00:00:00 2001 From: David Aurelio Date: Mon, 8 Feb 2016 11:18:25 -0800 Subject: [PATCH] Clean up Bundler Summary: The bundler class had duplicated code and parts that were hard to follow, because functions accepted heterogenous arguments, leading to a lot of conditionals. This commit tries to remove duplication between build steps of different type of bundles, and by accepting less different types of arguments. These differences are now handled further up the call stack. public Reviewed By: sebmarkbage Differential Revision: D2905807 fb-gh-sync-id: ef85ea0d461a9a06a4a64480e014a5324c4ef532 --- react-packager/src/Bundler/BundleBase.js | 12 +- .../src/Bundler/__tests__/Bundle-test.js | 13 +- react-packager/src/Bundler/index.js | 291 ++++++++---------- .../DependencyGraph/ResolutionResponse.js | 15 + react-packager/src/Server/index.js | 2 +- 5 files changed, 140 insertions(+), 193 deletions(-) diff --git a/react-packager/src/Bundler/BundleBase.js b/react-packager/src/Bundler/BundleBase.js index 79cbfefc..d3794a43 100644 --- a/react-packager/src/Bundler/BundleBase.js +++ b/react-packager/src/Bundler/BundleBase.js @@ -16,7 +16,7 @@ class BundleBase { this._finalized = false; this._modules = []; this._assets = []; - this._mainModuleId = this._mainModuleName = undefined; + this._mainModuleId = undefined; } isEmpty() { @@ -31,14 +31,6 @@ class BundleBase { this._mainModuleId = moduleId; } - getMainModuleName() { - return this._mainModuleName; - } - - setMainModuleName(moduleName) { - this._mainModuleName = moduleName; - } - addModule(module) { if (!module instanceof ModuleTransport) { throw new Error('Expeceted a ModuleTransport object'); @@ -89,7 +81,6 @@ class BundleBase { modules: this._modules, assets: this._assets, mainModuleId: this.getMainModuleId(), - mainModuleName: this.getMainModuleName(), }; } @@ -97,7 +88,6 @@ class BundleBase { bundle._assets = json.assets; bundle._modules = json.modules; bundle.setMainModuleId(json.mainModuleId); - bundle.setMainModuleName(json.mainModuleName); Object.freeze(bundle._modules); Object.seal(bundle._modules); diff --git a/react-packager/src/Bundler/__tests__/Bundle-test.js b/react-packager/src/Bundler/__tests__/Bundle-test.js index b4152a7b..23fe5762 100644 --- a/react-packager/src/Bundler/__tests__/Bundle-test.js +++ b/react-packager/src/Bundler/__tests__/Bundle-test.js @@ -312,28 +312,21 @@ describe('Bundle', () => { }); }); - describe('main module name and id:', function() { - it('keeps distinct module names and IDs', function() { + describe('main module id:', function() { + it('can save a main module ID', function() { const id = 'arbitrary module ID'; - const name = 'arbitrary module name'; bundle.setMainModuleId(id); - bundle.setMainModuleName(name); - expect(bundle.getMainModuleId()).toEqual(id); - expect(bundle.getMainModuleName()).toEqual(name); }); - it('can serialize and deserialize module ID and name', function() { + it('can serialize and deserialize the module ID', function() { const id = 'arbitrary module ID'; - const name = 'arbitrary module name'; bundle.setMainModuleId(id); - bundle.setMainModuleName(name); bundle.finalize({}); const deserialized = Bundle.fromJSON(bundle.toJSON()); expect(deserialized.getMainModuleId()).toEqual(id); - expect(deserialized.getMainModuleName()).toEqual(name); }); }); }); diff --git a/react-packager/src/Bundler/index.js b/react-packager/src/Bundler/index.js index fc7be66d..a07d73f7 100644 --- a/react-packager/src/Bundler/index.js +++ b/react-packager/src/Bundler/index.js @@ -29,6 +29,8 @@ const version = require('../../../../package.json').version; const sizeOf = Promise.denodeify(imageSize); const readFile = Promise.denodeify(fs.readFile); +const noop = () => {}; + const validateOpts = declareOpts({ projectRoots: { type: 'array', @@ -159,9 +161,12 @@ class Bundler { } bundle(options) { + const {dev, isUnbundle, platform} = options; + const moduleSystemDeps = + this._resolver.getModuleSystemDependencies({dev, isUnbundle, platform}); return this._bundle({ bundle: new Bundle(options.sourceMapUrl), - includeSystemDependencies: true, + moduleSystemDeps, ...options, }); } @@ -209,7 +214,7 @@ class Bundler { ); } - bundleForHMR(options) { + hmrBundle(options) { return this._bundle({ bundle: new HMRBundle({ sourceURLFn: this._sourceHMRURL.bind(this, options.platform), @@ -225,121 +230,50 @@ class Bundler { _bundle({ bundle, - modules, entryFile, runModule: runMainModule, runBeforeMainModule, dev: isDev, - includeSystemDependencies, platform, - unbundle: isUnbundle, - hot: hot, + moduleSystemDeps = [], + hot, entryModuleOnly, - resolutionResponse, + resolutionResponse }) { - let transformEventId; - const moduleSystemDeps = includeSystemDependencies - ? this._resolver.getModuleSystemDependencies( - { dev: isDev, platform, isUnbundle } - ) - : []; - - const findModules = () => { - const findEventId = Activity.startEvent('find dependencies'); - return this.getDependencies(entryFile, isDev, platform).then(response => { - Activity.endEvent(findEventId); - bundle.setMainModuleId(response.mainModuleId); - bundle.setMainModuleName(response.mainModuleId); - if (!entryModuleOnly && bundle.setNumPrependedModules) { - bundle.setNumPrependedModules( - response.numPrependedDependencies + moduleSystemDeps.length - ); - } - - return { - response, - modulesToProcess: response.dependencies, - }; - }); - }; - - const useProvidedModules = () => { - const moduleId = this._resolver.getModuleForPath(entryFile); - bundle.setMainModuleId(moduleId); - bundle.setMainModuleName(moduleId); - return Promise.resolve({ - response: resolutionResponse, - modulesToProcess: modules - }); - }; - - return ( - modules ? useProvidedModules() : findModules() - ).then(({response, modulesToProcess}) => { - - transformEventId = Activity.startEvent('transform'); - - let dependencies; + const onResolutionResponse = response => { + bundle.setMainModuleId(response.mainModuleId); + if (bundle.setNumPrependedModules) { + bundle.setNumPrependedModules( + response.numPrependedDependencies + moduleSystemDeps.length + ); + } if (entryModuleOnly) { - dependencies = response.dependencies.filter(module => + response.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; - dependencies = moduleSystemDeps.concat(modulesToProcess); + response.dependencies = moduleSystemDeps.concat(response.dependencies); } - - let bar; - if (process.stdout.isTTY) { - bar = new ProgressBar('transforming [:bar] :percent :current/:total', { - complete: '=', - incomplete: ' ', - width: 40, - total: dependencies.length, - }); - } - - return Promise.all( - dependencies.map( - module => { - return this._transformModule( - bundle, - module, - platform, - isDev, - hot, - ).then(transformed => { - if (bar) { - bar.tick(); - } - - return { - module, - transformed, - }; - }); - } + }; + const finalizeBundle = ({bundle, transformedModules, response}) => + Promise.all( + transformedModules.map(({module, transformed}) => + bundle.addModule(this._resolver, response, module, transformed) ) - ).then(transformedModules => Promise.all( - transformedModules.map(({module, transformed}) => { - return bundle.addModule( - this._resolver, - response, - module, - transformed, - ); - }) - )); - }).then(() => { - Activity.endEvent(transformEventId); - bundle.finalize({runBeforeMainModule, runMainModule}); - return bundle; + ).then(() => { + bundle.finalize({runBeforeMainModule, runMainModule}); + return bundle; + }); + + return this._buildBundle({ + entryFile, + isDev, + platform, + bundle, + hot, + resolutionResponse, + onResolutionResponse, + finalizeBundle, }); } @@ -351,78 +285,86 @@ class Bundler { dev: isDev, platform, }) { - const bundle = new PrepackBundle(sourceMapUrl); - const findEventId = Activity.startEvent('find dependencies'); - let transformEventId; - let mainModuleId; - - return this.getDependencies(entryFile, isDev, platform).then((response) => { - Activity.endEvent(findEventId); - transformEventId = Activity.startEvent('transform'); - - let bar; - if (process.stdout.isTTY) { - bar = new ProgressBar('transforming [:bar] :percent :current/:total', { - complete: '=', - incomplete: ' ', - width: 40, - total: response.dependencies.length, + const onModuleTransformed = ({module, transformed, response, bundle}) => { + const deps = Object.create(null); + const pairs = response.getResolvedDependencyPairs(module); + if (pairs) { + pairs.forEach(pair => { + deps[pair[0]] = pair[1].path; }); } - mainModuleId = response.mainModuleId; - - return Promise.all( - response.dependencies.map( - module => this._transformModule( - bundle, - module, - platform, - isDev, - ).then(transformed => { - if (bar) { - bar.tick(); - } - - var deps = Object.create(null); - var pairs = response.getResolvedDependencyPairs(module); - if (pairs) { - pairs.forEach(pair => { - deps[pair[0]] = pair[1].path; - }); - } - - return module.getName().then(name => { - bundle.addModule(name, transformed, deps, module.isPolyfill()); - }); - }) - ) - ); - }).then(() => { - Activity.endEvent(transformEventId); - bundle.finalize({runBeforeMainModule, runMainModule, mainModuleId }); + return module.getName().then(name => { + bundle.addModule(name, transformed, deps, module.isPolyfill()); + }); + }; + const finalizeBundle = ({bundle, response}) => { + const {mainModuleId} = response; + bundle.finalize({runBeforeMainModule, runMainModule, mainModuleId}); return bundle; + }; + + return this._buildBundle({ + entryFile, + isDev, + platform, + onModuleTransformed, + finalizeBundle, + bundle: new PrepackBundle(sourceMapUrl), }); } - _transformModuleForHMR(module, platform) { - if (module.isAsset()) { - return this._generateAssetObjAndCode(module, platform).then( - ({asset, code}) => { - return { - code, - }; - } - ); - } else { - // TODO(martinb): pass non null main (t9527509) - return this._getTransformOptions( - {main: null, dev: true, platform: 'ios'}, // TODO(martinb): avoid hard-coding platform - {hot: true}, - ).then(options => { - return this._transformer.loadFileAndTransform(module.path, options); - }); + _buildBundle({ + entryFile, + isDev, + platform, + bundle, + hot, + resolutionResponse, + onResolutionResponse = noop, + onModuleTransformed = noop, + finalizeBundle = noop, + }) { + const findEventId = Activity.startEvent('find dependencies'); + if (!resolutionResponse) { + resolutionResponse = this.getDependencies(entryFile, isDev, platform); } + + return Promise.resolve(resolutionResponse).then(response => { + Activity.endEvent(findEventId); + onResolutionResponse(response); + + const transformEventId = Activity.startEvent('transform'); + const bar = process.stdout.isTTY + ? new ProgressBar('transforming [:bar] :percent :current/:total', { + complete: '=', + incomplete: ' ', + width: 40, + total: response.dependencies.length, + }) + : {tick() {}}; + const transformPromises = + response.dependencies.map(module => + this._transformModule({ + mainModuleName: response.mainModuleId, + bundle, + module, + platform, + dev: isDev, + hot + }).then(transformed => { + bar.tick(); + onModuleTransformed({module, transformed, response, bundle}); + return {module, transformed}; + }) + ); + return Promise.all(transformPromises).then(transformedModules => { + Activity.endEvent(transformEventId); + return Promise + .resolve(finalizeBundle({bundle, transformedModules, response})) + .then(() => bundle); + }); + }); } invalidateFile(filePath) { @@ -489,7 +431,14 @@ class Bundler { ); } - _transformModule(bundle, module, platform = null, dev = true, hot = false) { + _transformModule({ + bundle, + module, + mainModuleName, + platform = null, + dev = true, + hot = false, + }) { if (module.isAsset_DEPRECATED()) { return this._generateAssetModule_DEPRECATED(bundle, module); } else if (module.isAsset()) { @@ -499,12 +448,12 @@ class Bundler { } else { return this._getTransformOptions( { - bundleEntry: bundle.getMainModuleName(), + bundleEntry: mainModuleName, platform: platform, dev: dev, modulePath: module.path, }, - {hot: hot}, + {hot}, ).then(options => { return this._transformer.loadFileAndTransform( path.resolve(module.path), diff --git a/react-packager/src/DependencyResolver/DependencyGraph/ResolutionResponse.js b/react-packager/src/DependencyResolver/DependencyGraph/ResolutionResponse.js index 7fbafde8..c13895fb 100644 --- a/react-packager/src/DependencyResolver/DependencyGraph/ResolutionResponse.js +++ b/react-packager/src/DependencyResolver/DependencyGraph/ResolutionResponse.js @@ -19,6 +19,21 @@ class ResolutionResponse { this._finalized = false; } + copy(properties) { + const { + dependencies = this.dependencies, + asyncDependencies = this.asyncDependencies, + mainModuleId = this.mainModuleId, + mocks = this.mocks, + } = properties; + return Object.assign(new this.constructor(), this, { + dependencies, + asyncDependencies, + mainModuleId, + mocks, + }); + } + _assertNotFinalized() { if (this._finalized) { throw new Error('Attempted to mutate finalized response.'); diff --git a/react-packager/src/Server/index.js b/react-packager/src/Server/index.js index f8c252a3..0146e7d6 100644 --- a/react-packager/src/Server/index.js +++ b/react-packager/src/Server/index.js @@ -241,7 +241,7 @@ class Server { } buildBundleForHMR(modules) { - return this._bundler.bundleForHMR(modules); + return this._bundler.hmrBundle(modules); } getShallowDependencies(entryFile) {