From ed55e0a53f33d9d57fd25781fc49f2909381b336 Mon Sep 17 00:00:00 2001 From: Rafael Oleza Date: Wed, 8 Nov 2017 03:52:50 -0800 Subject: [PATCH] Remove cyclic dependency when getting the transform options Reviewed By: davidaurelio Differential Revision: D6231766 fbshipit-source-id: ae200a4507febcd47bcc636d0e62f01eb6a93477 --- packages/metro-bundler/src/Bundler/index.js | 73 ++++++++++++++--- .../src/DeltaBundler/DeltaCalculator.js | 64 +++++++++++---- .../__tests__/DeltaCalculator-test.js | 78 ++++++++++++++++++- 3 files changed, 187 insertions(+), 28 deletions(-) diff --git a/packages/metro-bundler/src/Bundler/index.js b/packages/metro-bundler/src/Bundler/index.js index 32077e1f..1bd438f2 100644 --- a/packages/metro-bundler/src/Bundler/index.js +++ b/packages/metro-bundler/src/Bundler/index.js @@ -48,10 +48,14 @@ export type BundlingOptions = {| +transformer: JSTransformerOptions, |}; +type TransformOptions = {| + +inlineRequires: {+blacklist: {[string]: true}} | boolean, +|}; + export type ExtraTransformOptions = { +preloadedModules?: {[path: string]: true} | false, +ramGroups?: Array, - +transform?: {+inlineRequires?: {+blacklist: {[string]: true}} | boolean}, + +transform?: TransformOptions, }; export type GetTransformOptionsOpts = {| @@ -594,14 +598,17 @@ class Bundler { transformerOptions?: JSTransformerOptions, }): Promise> { if (!transformerOptions) { - transformerOptions = (await this.getTransformOptions(rootEntryFile, { - dev, - generateSourceMaps, - hot, - minify, - platform, - prependPolyfills: false, - })).transformer; + transformerOptions = (await this._getLegacyTransformOptions_Do_Not_Use( + rootEntryFile, + { + dev, + generateSourceMaps, + hot, + minify, + platform, + prependPolyfills: false, + }, + )).transformer; } const notNullOptions = transformerOptions; @@ -642,7 +649,7 @@ class Bundler { +prependPolyfills: boolean, onProgress?: ?(finishedModules: number, totalModules: number) => mixed, }): Promise> { - const bundlingOptions: BundlingOptions = await this.getTransformOptions( + const bundlingOptions: BundlingOptions = await this._getLegacyTransformOptions_Do_Not_Use( rootEntryFile, { dev, @@ -845,7 +852,51 @@ class Bundler { }); } - async getTransformOptions( + /** + * Returns the transform options related to a specific entry file, by calling + * the config parameter getTransformOptions(). + */ + async getTransformOptionsForEntryFile( + entryFile: string, + options: {dev: boolean, platform: ?string}, + getDependencies: string => Promise>, + ): Promise { + if (!this._getTransformOptions) { + return { + inlineRequires: false, + }; + } + + const {transform} = await this._getTransformOptions( + [entryFile], + {dev: options.dev, hot: true, platform: options.platform}, + getDependencies, + ); + + return transform || {inlineRequires: false}; + } + + /* + * Helper method to return the global transform options that are kept in the + * Bundler. + */ + getGlobalTransformOptions(): { + enableBabelRCLookup: boolean, + projectRoot: string, + } { + return { + enableBabelRCLookup: this._opts.enableBabelRCLookup, + projectRoot: this._projectRoots[0], + }; + } + + /* + * Old logic to get the transform options, which automatically calculates + * the dependencies of the inlineRequires and preloadedModules params. + * + * TODO: Remove this. + */ + async _getLegacyTransformOptions_Do_Not_Use( mainModuleName: string, options: {| dev: boolean, diff --git a/packages/metro-bundler/src/DeltaBundler/DeltaCalculator.js b/packages/metro-bundler/src/DeltaBundler/DeltaCalculator.js index 4e5098f5..74692b48 100644 --- a/packages/metro-bundler/src/DeltaBundler/DeltaCalculator.js +++ b/packages/metro-bundler/src/DeltaBundler/DeltaCalculator.js @@ -137,27 +137,65 @@ class DeltaCalculator extends EventEmitter { } /** - * Returns the options options object that is used by ResoltionRequest to - * read all the modules. This can be used by external objects to read again + * Returns the options object that is used by the transformer to parse + * all the modules. This can be used by external objects to read again * any module very fast (since the options object instance will be the same). */ async getTransformerOptions(): Promise { if (!this._transformerOptions) { - this._transformerOptions = (await this._bundler.getTransformOptions( - this._options.entryFile, - { - dev: this._options.dev, - generateSourceMaps: false, - hot: this._options.hot, - minify: this._options.minify, - platform: this._options.platform, - prependPolyfills: false, - }, - )).transformer; + this._transformerOptions = await this._calcTransformerOptions(); } return this._transformerOptions; } + async _calcTransformerOptions(): Promise { + const { + enableBabelRCLookup, + projectRoot, + } = this._bundler.getGlobalTransformOptions(); + + const transformOptionsForBlacklist = { + dev: this._options.dev, + minify: this._options.minify, + platform: this._options.platform, + transform: { + enableBabelRCLookup, + dev: this._options.dev, + generateSourceMaps: this._options.generateSourceMaps, + hot: this._options.hot, + inlineRequires: false, + platform: this._options.platform, + projectRoot, + }, + }; + + const { + inlineRequires, + } = await this._bundler.getTransformOptionsForEntryFile( + this._options.entryFile, + {dev: this._options.dev, platform: this._options.platform}, + async path => { + const {added} = await initialTraverseDependencies( + path, + this._dependencyGraph, + transformOptionsForBlacklist, + new Map(), + ); + + return [path, ...added]; + }, + ); + + // $FlowFixMe flow does not recognize well Object.assign() return types. + return { + ...transformOptionsForBlacklist, + transform: { + ...transformOptionsForBlacklist.transform, + inlineRequires: inlineRequires || false, + }, + }; + } + /** * Returns all the dependency edges from the graph. Each edge contains the * needed information to do the traversing (dependencies, inverseDependencies) diff --git a/packages/metro-bundler/src/DeltaBundler/__tests__/DeltaCalculator-test.js b/packages/metro-bundler/src/DeltaBundler/__tests__/DeltaCalculator-test.js index 83fda8e0..9066ef91 100644 --- a/packages/metro-bundler/src/DeltaBundler/__tests__/DeltaCalculator-test.js +++ b/packages/metro-bundler/src/DeltaBundler/__tests__/DeltaCalculator-test.js @@ -102,12 +102,17 @@ describe('DeltaCalculator', () => { }, ); - Bundler.prototype.getTransformOptions.mockImplementation(async () => { - return { - transformer: {}, - }; + Bundler.prototype.getGlobalTransformOptions.mockReturnValue({ + enableBabelRCLookup: false, + projectRoot: '/foo', }); + Bundler.prototype.getTransformOptionsForEntryFile.mockReturnValue( + Promise.resolve({ + inlineRequires: false, + }), + ); + deltaCalculator = new DeltaCalculator( bundlerMock, dependencyGraph, @@ -291,4 +296,69 @@ describe('DeltaCalculator', () => { expect(traverseDependencies.mock.calls[0][0]).toEqual(['/bundle']); }); + + describe('getTransformerOptions()', () => { + it('should calculate the transform options correctly', async () => { + expect(await deltaCalculator.getTransformerOptions()).toEqual({ + dev: true, + minify: false, + platform: 'ios', + transform: { + dev: true, + enableBabelRCLookup: false, + generateSourceMaps: false, + hot: true, + inlineRequires: false, + platform: 'ios', + projectRoot: '/foo', + }, + }); + }); + + it('should handle inlineRequires=true correctly', async () => { + Bundler.prototype.getTransformOptionsForEntryFile.mockReturnValue( + Promise.resolve({ + inlineRequires: true, + }), + ); + + expect(await deltaCalculator.getTransformerOptions()).toEqual({ + dev: true, + minify: false, + platform: 'ios', + transform: { + dev: true, + enableBabelRCLookup: false, + generateSourceMaps: false, + hot: true, + inlineRequires: true, + platform: 'ios', + projectRoot: '/foo', + }, + }); + }); + + it('should handle an inline requires blacklist correctly', async () => { + Bundler.prototype.getTransformOptionsForEntryFile.mockReturnValue( + Promise.resolve({ + inlineRequires: {blacklist: {'/bar': true, '/baz': true}}, + }), + ); + + expect(await deltaCalculator.getTransformerOptions()).toEqual({ + dev: true, + minify: false, + platform: 'ios', + transform: { + dev: true, + enableBabelRCLookup: false, + generateSourceMaps: false, + hot: true, + inlineRequires: {blacklist: {'/bar': true, '/baz': true}}, + platform: 'ios', + projectRoot: '/foo', + }, + }); + }); + }); });