From 5d2a4578809ee6d6a6198ab8a6e8a9e807fb23d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Bigio?= Date: Fri, 29 Jan 2016 10:14:37 -0800 Subject: [PATCH] Make HMR faster Summary: public At the moment, when the user changes a file we end up pulling the dependencies of the entry point to build the bundle. This could take a long time if the bundle is big. To avoid it, lets introduce a new parameter to `getDependencies` to be able to avoid processing the modules recursively and reuse the resolution responseto build the bundle. Reviewed By: davidaurelio Differential Revision: D2862850 fb-gh-sync-id: b8ae2b811a8ae9aec5612f9655d1c762671ce730 --- .../src/Bundler/__tests__/Bundler-test.js | 11 ++-- react-packager/src/Bundler/index.js | 63 ++++++++++++++----- .../DependencyGraph/ResolutionRequest.js | 6 +- .../DependencyGraph/index.js | 8 ++- .../src/Resolver/__tests__/Resolver-test.js | 3 +- react-packager/src/Resolver/index.js | 22 ++++--- react-packager/src/Server/index.js | 5 ++ 7 files changed, 86 insertions(+), 32 deletions(-) diff --git a/react-packager/src/Bundler/__tests__/Bundler-test.js b/react-packager/src/Bundler/__tests__/Bundler-test.js index 467cf90d..5ed7d658 100644 --- a/react-packager/src/Bundler/__tests__/Bundler-test.js +++ b/react-packager/src/Bundler/__tests__/Bundler-test.js @@ -206,11 +206,12 @@ describe('Bundler', function() { }); pit('gets the list of dependencies from the resolver', function() { - return bundler.getDependencies('/root/foo.js', true) - .then( - () => expect(getDependencies) - .toBeCalledWith('/root/foo.js', { dev: true }) - ); + return bundler.getDependencies('/root/foo.js', true).then(() => + expect(getDependencies).toBeCalledWith( + '/root/foo.js', + { dev: true, recursive: true }, + ) + ); }); describe('getOrderedDependencyPaths', () => { diff --git a/react-packager/src/Bundler/index.js b/react-packager/src/Bundler/index.js index b0ccc235..d03fdb89 100644 --- a/react-packager/src/Bundler/index.js +++ b/react-packager/src/Bundler/index.js @@ -235,14 +235,48 @@ class Bundler { unbundle: isUnbundle, hot: hot, entryModuleOnly, + resolutionResponse, }) { - const findEventId = Activity.startEvent('find dependencies'); 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}) => { - return this.getDependencies(entryFile, isDev, platform).then((response) => { - Activity.endEvent(findEventId); - bundle.setMainModuleId(response.mainModuleId); - bundle.setMainModuleName(response.mainModuleId); transformEventId = Activity.startEvent('transform'); let dependencies; @@ -259,10 +293,6 @@ class Bundler { const modulesToProcess = modules || response.dependencies; dependencies = moduleSystemDeps.concat(modulesToProcess); - - bundle.setNumPrependedModules && bundle.setNumPrependedModules( - response.numPrependedDependencies + moduleSystemDeps.length - ); } let bar; @@ -280,7 +310,6 @@ class Bundler { module => { return this._transformModule( bundle, - response, module, platform, isDev, @@ -347,7 +376,6 @@ class Bundler { response.dependencies.map( module => this._transformModule( bundle, - response, module, platform, isDev, @@ -418,8 +446,15 @@ class Bundler { return this._resolver.getModuleForPath(entryFile); } - getDependencies(main, isDev, platform) { - return this._resolver.getDependencies(main, { dev: isDev, platform }); + getDependencies(main, isDev, platform, recursive = true) { + return this._resolver.getDependencies( + main, + { + dev: isDev, + platform, + recursive, + }, + ); } getOrderedDependencyPaths({ entryFile, dev, platform }) { @@ -454,7 +489,7 @@ class Bundler { ); } - _transformModule(bundle, response, module, platform = null, dev = true, hot = false) { + _transformModule(bundle, module, platform = null, dev = true, hot = false) { if (module.isAsset_DEPRECATED()) { return this._generateAssetModule_DEPRECATED(bundle, module); } else if (module.isAsset()) { diff --git a/react-packager/src/DependencyResolver/DependencyGraph/ResolutionRequest.js b/react-packager/src/DependencyResolver/DependencyGraph/ResolutionRequest.js index 58dcc7d1..149cab01 100644 --- a/react-packager/src/DependencyResolver/DependencyGraph/ResolutionRequest.js +++ b/react-packager/src/DependencyResolver/DependencyGraph/ResolutionRequest.js @@ -103,7 +103,7 @@ class ResolutionRequest { ); } - getOrderedDependencies(response, mocksPattern) { + getOrderedDependencies(response, mocksPattern, recursive = true) { return this._getAllMocks(mocksPattern).then(allMocks => { const entry = this._moduleCache.getModule(this._entryPath); const mocks = Object.create(null); @@ -163,7 +163,9 @@ class ResolutionRequest { p = p.then(() => { if (!visited[modDep.hash()]) { visited[modDep.hash()] = true; - return collect(modDep); + if (recursive) { + return collect(modDep); + } } return null; }); diff --git a/react-packager/src/DependencyResolver/DependencyGraph/index.js b/react-packager/src/DependencyResolver/DependencyGraph/index.js index 023fca50..273b9b58 100644 --- a/react-packager/src/DependencyResolver/DependencyGraph/index.js +++ b/react-packager/src/DependencyResolver/DependencyGraph/index.js @@ -158,7 +158,7 @@ class DependencyGraph { return this._moduleCache.getModule(entryFile); } - getDependencies(entryPath, platform) { + getDependencies(entryPath, platform, recursive = true) { return this.load().then(() => { platform = this._getRequestPlatform(entryPath, platform); const absPath = this._getAbsolutePath(entryPath); @@ -177,7 +177,11 @@ class DependencyGraph { const response = new ResolutionResponse(); return Promise.all([ - req.getOrderedDependencies(response, this._opts.mocksPattern), + req.getOrderedDependencies( + response, + this._opts.mocksPattern, + recursive, + ), req.getAsyncDependencies(response), ]).then(() => response); }); diff --git a/react-packager/src/Resolver/__tests__/Resolver-test.js b/react-packager/src/Resolver/__tests__/Resolver-test.js index b542d43b..962bba96 100644 --- a/react-packager/src/Resolver/__tests__/Resolver-test.js +++ b/react-packager/src/Resolver/__tests__/Resolver-test.js @@ -186,7 +186,8 @@ describe('Resolver', function() { return depResolver.getDependencies('/root/index.js', { dev: true }) .then(function(result) { expect(result.mainModuleId).toEqual('index'); - expect(DependencyGraph.mock.instances[0].getDependencies).toBeCalledWith('/root/index.js', undefined); + expect(DependencyGraph.mock.instances[0].getDependencies) + .toBeCalledWith('/root/index.js', undefined, true); expect(result.dependencies[0]).toBe(Polyfill.mock.instances[0]); expect(result.dependencies[result.dependencies.length - 1]) .toBe(module); diff --git a/react-packager/src/Resolver/index.js b/react-packager/src/Resolver/index.js index 4cb4707f..4f3444ec 100644 --- a/react-packager/src/Resolver/index.js +++ b/react-packager/src/Resolver/index.js @@ -64,6 +64,10 @@ const getDependenciesValidateOpts = declareOpts({ type: 'boolean', default: false }, + recursive: { + type: 'boolean', + default: true, + }, }); class Resolver { @@ -116,15 +120,17 @@ class Resolver { getDependencies(main, options) { const opts = getDependenciesValidateOpts(options); - return this._depGraph.getDependencies(main, opts.platform).then( - resolutionResponse => { - this._getPolyfillDependencies().reverse().forEach( - polyfill => resolutionResponse.prependDependency(polyfill) - ); + return this._depGraph.getDependencies( + main, + opts.platform, + opts.recursive, + ).then(resolutionResponse => { + this._getPolyfillDependencies().reverse().forEach( + polyfill => resolutionResponse.prependDependency(polyfill) + ); - return resolutionResponse.finalize(); - } - ); + return resolutionResponse.finalize(); + }); } getModuleSystemDependencies(options) { diff --git a/react-packager/src/Server/index.js b/react-packager/src/Server/index.js index 559fb302..defbf5a5 100644 --- a/react-packager/src/Server/index.js +++ b/react-packager/src/Server/index.js @@ -137,6 +137,10 @@ const dependencyOpts = declareOpts({ type: 'string', required: true, }, + recursive: { + type: 'boolean', + default: true, + }, }); class Server { @@ -269,6 +273,7 @@ class Server { opts.entryFile, opts.dev, opts.platform, + opts.recursive, ); }); }