Remove irrelevant options from cache key

Reviewed By: jeanlauliac

Differential Revision: D4938131

fbshipit-source-id: 88b686bc5ee6946297e1fd1b91d46fa618f0d9d7

packages/metro-bundler/src/Bundler/__tests__/Bundler-test.js

@@ -134,7 +134,7 @@ describe('Bundler', function() {
       Promise.resolve({
         mainModuleId: 'foo',
         dependencies: modules,
-        transformOptions,
+        options: transformOptions,
         getModuleId: () => 123,
         getResolvedDependencyPairs: () => [],
       })
@@ -272,18 +272,20 @@ describe('Bundler', function() {
         '/root/foo.js',
         {dev: true, platform: undefined, recursive: true},
         {
-          dev: true,
-          minify: false,
-          platform: undefined,
-          transform: {
+          preloadedModules: undefined,
+          ramGroups: undefined,
+          transformer: {
             dev: true,
-            generateSourceMaps: false,
-            hot: false,
-            inlineRequires: false,
+            minify: false,
             platform: undefined,
-            preloadedModules: undefined,
-            projectRoots,
-            ramGroups: undefined,
+            transform: {
+              dev: true,
+              generateSourceMaps: false,
+              hot: false,
+              inlineRequires: false,
+              platform: undefined,
+              projectRoots,
+            },
           },
         },
       ])

packages/metro-bundler/src/Bundler/index.js

@@ -40,13 +40,16 @@ const VERSION = require('../../package.json').version;
 import type AssetServer from '../AssetServer';
 import type Module, {HasteImpl} from '../node-haste/Module';
 import type ResolutionResponse from '../node-haste/DependencyGraph/ResolutionResponse';
-import type {
-  Options as JSTransformerOptions,
-  TransformOptions,
-} from '../JSTransformer/worker/worker';
+import type {Options as JSTransformerOptions} from '../JSTransformer/worker/worker';
 import type {Reporter} from '../lib/reporting';
 import type {GlobalTransformCache} from '../lib/GlobalTransformCache';
 
+export type BundlingOptions = {|
+  +preloadedModules: ?{[string]: true} | false,
+  +ramGroups: ?Array<string>,
+  +transformer: JSTransformerOptions,
+|};
+
 export type ExtraTransformOptions = {|
   +inlineRequires?: {+blacklist: {[string]: true}} | boolean,
   +preloadedModules?: {[path: string]: true} | false,
@@ -324,12 +327,12 @@ class Bundler {
     moduleSystemDeps?: Array<Module>,
     onProgress?: () => void,
     platform?: ?string,
-    resolutionResponse?: ResolutionResponse<Module>,
+    resolutionResponse?: ResolutionResponse<Module, BundlingOptions>,
     runBeforeMainModule?: boolean,
     runModule?: boolean,
     unbundle?: boolean,
   }) {
-    const onResolutionResponse = (response: ResolutionResponse<Module>) => {
+    const onResolutionResponse = (response: ResolutionResponse<Module, BundlingOptions>) => {
       /* $FlowFixMe: looks like ResolutionResponse is monkey-patched
        * with `getModuleId`. */
       bundle.setMainModuleId(response.getModuleId(getMainModule(response)));
@@ -344,7 +347,7 @@ class Bundler {
     const finalizeBundle = ({bundle: finalBundle, transformedModules, response, modulesByName}: {
       bundle: Bundle,
       transformedModules: Array<{module: Module, transformed: ModuleTransport}>,
-      response: ResolutionResponse<Module>,
+      response: ResolutionResponse<Module, BundlingOptions>,
       modulesByName: {[name: string]: Module},
     }) =>
       this._resolverPromise.then(resolver => Promise.all(
@@ -427,7 +430,7 @@ class Bundler {
     return Promise.all(
       [this._resolverPromise, resolutionResponse],
     ).then(([resolver, response]) => {
-      bundle.setRamGroups(response.transformOptions.transform.ramGroups);
+      bundle.setRamGroups(response.options.ramGroups);
 
       log(createActionEndEntry(transformingFilesLogEntry));
       onResolutionResponse(response);
@@ -452,7 +455,7 @@ class Bundler {
           bundle,
           entryFilePath,
           assetPlugins,
-          transformOptions: response.transformOptions,
+          options: response.options,
           /* $FlowFixMe: `getModuleId` is monkey-patched */
           getModuleId: (response.getModuleId: () => number),
           dependencyPairs: response.getResolvedDependencyPairs(module),
@@ -495,30 +498,24 @@ class Bundler {
       entryFile,
       {
         dev,
-        platform,
-        hot,
         generateSourceMaps,
+        hot,
+        minify,
+        platform,
         projectRoots: this._projectRoots,
       },
-    ).then(transformSpecificOptions => {
-      const transformOptions = {
-        minify,
-        dev,
-        platform,
-        transform: transformSpecificOptions,
-      };
-
-      return this._resolverPromise.then(
-        resolver => resolver.getShallowDependencies(entryFile, transformOptions),
-      );
-    });
+    ).then(bundlingOptions =>
+      this._resolverPromise.then(resolver =>
+        resolver.getShallowDependencies(entryFile, bundlingOptions.transformer),
+      )
+    );
   }
 
   getModuleForPath(entryFile: string): Promise<Module> {
     return this._resolverPromise.then(resolver => resolver.getModuleForPath(entryFile));
   }
 
-  getDependencies({
+  async getDependencies({
     entryFile,
     platform,
     dev = true,
@@ -538,32 +535,28 @@ class Bundler {
     generateSourceMaps?: boolean,
     isolateModuleIDs?: boolean,
     onProgress?: ?(finishedModules: number, totalModules: number) => mixed,
-  }) {
-    return this.getTransformOptions(
+  }): Promise<ResolutionResponse<Module, BundlingOptions>> {
+    const bundlingOptions: BundlingOptions = await this.getTransformOptions(
       entryFile,
       {
         dev,
         platform,
         hot,
         generateSourceMaps,
+        minify,
         projectRoots: this._projectRoots,
       },
-    ).then(transformSpecificOptions => {
-      const transformOptions = {
-        minify,
-        dev,
-        platform,
-        transform: transformSpecificOptions,
-      };
+    );
 
-      return this._resolverPromise.then(resolver => resolver.getDependencies(
-        entryFile,
-        {dev, platform, recursive},
-        transformOptions,
-        onProgress,
-        isolateModuleIDs ? createModuleIdFactory() : this._getModuleId,
-      ));
-    });
+    const resolver = await this._resolverPromise;
+    const response = await resolver.getDependencies(
+      entryFile,
+      {dev, platform, recursive},
+      bundlingOptions,
+      onProgress,
+      isolateModuleIDs ? createModuleIdFactory() : this._getModuleId,
+    );
+    return response;
   }
 
   getOrderedDependencyPaths({entryFile, dev, platform}: {
@@ -606,7 +599,7 @@ class Bundler {
     module,
     bundle,
     entryFilePath,
-    transformOptions,
+    options,
     getModuleId,
     dependencyPairs,
     assetPlugins,
@@ -614,13 +607,14 @@ class Bundler {
     module: Module,
     bundle: Bundle,
     entryFilePath: string,
-    transformOptions: JSTransformerOptions,
+    options: BundlingOptions,
     getModuleId: () => number,
     dependencyPairs: Array<[mixed, {path: string}]>,
     assetPlugins: Array<string>,
   }): Promise<ModuleTransport> {
     let moduleTransport;
     const moduleId = getModuleId(module);
+    const transformOptions = options.transformer;
 
     if (module.isAsset()) {
       moduleTransport = this._generateAssetModule(
@@ -637,7 +631,7 @@ class Bundler {
     ]).then((
       [name, {code, dependencies, dependencyOffsets, map, source}]
     ) => {
-      const {preloadedModules} = transformOptions.transform;
+      const {preloadedModules} = options;
       const preloaded =
         module.path === entryFilePath ||
         module.isPolyfill() ||
@@ -777,10 +771,11 @@ class Bundler {
       dev: boolean,
       generateSourceMaps: boolean,
       hot: boolean,
+      minify: boolean,
       platform: string,
       projectRoots: Array<string>,
     |},
-    ): Promise<TransformOptions> {
+    ): Promise<BundlingOptions> {
     const getDependencies = (entryFile: string) =>
       this.getDependencies({...options, entryFile})
         .then(r => r.dependencies.map(d => d.path));
@@ -790,13 +785,20 @@ class Bundler {
       ? await this._getTransformOptions(mainModuleName, {dev, hot, platform}, getDependencies)
       : {};
     return {
-      dev,
-      generateSourceMaps: options.generateSourceMaps,
-      hot,
-      inlineRequires: extraOptions.inlineRequires || false,
-      platform,
+      transformer: {
+        dev,
+        minify: options.minify,
+        platform,
+        transform: {
+          dev,
+          generateSourceMaps: options.generateSourceMaps,
+          hot,
+          inlineRequires: extraOptions.inlineRequires || false,
+          platform,
+          projectRoots: options.projectRoots,
+        }
+      },
       preloadedModules: extraOptions.preloadedModules,
-      projectRoots: options.projectRoots,
       ramGroups: extraOptions.ramGroups,
     };
   }

packages/metro-bundler/src/JSTransformer/worker/worker.js

@@ -41,9 +41,7 @@ export type TransformOptions = {|
   +hot: boolean,
   +inlineRequires: {+blacklist: {[string]: true}} | boolean,
   +platform: string,
-  +preloadedModules: ?{[string]: true} | false,
   +projectRoots: Array<string>,
-  +ramGroups: ?Array<string>,
 |};
 
 export type Options = {|

packages/metro-bundler/src/Resolver/__tests__/Resolver-test.js

@@ -109,7 +109,7 @@ describe('Resolver', function() {
           expect(DependencyGraph.prototype.getDependencies).toBeCalledWith({
             entryPath: entry,
             platform,
-            transformOptions,
+            options: transformOptions,
             recursive: true,
           });
         });

packages/metro-bundler/src/Resolver/index.js

@@ -17,17 +17,18 @@ const defaults = require('../../defaults');
 const pathJoin = require('path').join;
 
 import type ResolutionResponse from '../node-haste/DependencyGraph/ResolutionResponse';
-import type Module, {HasteImpl} from '../node-haste/Module';
+import type Module, {HasteImpl, TransformCode} from '../node-haste/Module';
 import type {SourceMap} from '../lib/SourceMap';
-import type {Options as TransformOptions} from '../JSTransformer/worker/worker';
+import type {Options as JSTransformerOptions} from '../JSTransformer/worker/worker';
 import type {Reporter} from '../lib/reporting';
-import type {TransformCode} from '../node-haste/Module';
 import type {GetTransformCacheKey} from '../lib/TransformCache';
 import type {GlobalTransformCache} from '../lib/GlobalTransformCache';
 
 type MinifyCode = (filePath: string, code: string, map: SourceMap) =>
   Promise<{code: string, map: SourceMap}>;
 
+type ContainsTransformerOptions = {+transformer: JSTransformerOptions}
+
 type Options = {|
   +assetExts: Array<string>,
   +blacklistRE?: RegExp,
@@ -82,7 +83,7 @@ class Resolver {
 
   getShallowDependencies(
     entryFile: string,
-    transformOptions: TransformOptions,
+    transformOptions: JSTransformerOptions,
   ): Promise<Array<Module>> {
     return this._depGraph.getShallowDependencies(entryFile, transformOptions);
   }
@@ -91,18 +92,18 @@ class Resolver {
     return this._depGraph.getModuleForPath(entryFile);
   }
 
-  getDependencies(
+  getDependencies<T: ContainsTransformerOptions>(
     entryPath: string,
     options: {platform: string, recursive?: boolean},
-    transformOptions: TransformOptions,
+    bundlingOptions: T,
     onProgress?: ?(finishedModules: number, totalModules: number) => mixed,
     getModuleId: mixed,
-  ): Promise<ResolutionResponse<Module>> {
+  ): Promise<ResolutionResponse<Module, T>> {
     const {platform, recursive = true} = options;
     return this._depGraph.getDependencies({
       entryPath,
       platform,
-      transformOptions,
+      options: bundlingOptions,
       recursive,
       onProgress,
     }).then(resolutionResponse => {
@@ -146,8 +147,8 @@ class Resolver {
     );
   }
 
-  resolveRequires(
-    resolutionResponse: ResolutionResponse<Module>,
+  resolveRequires<T: ContainsTransformerOptions>(
+    resolutionResponse: ResolutionResponse<Module, T>,
     module: Module,
     code: string,
     dependencyOffsets: Array<number> = [],
@@ -181,7 +182,7 @@ class Resolver {
     ).join('');
   }
 
-  wrapModule({
+  wrapModule<T: ContainsTransformerOptions>({
     resolutionResponse,
     module,
     name,
@@ -191,7 +192,7 @@ class Resolver {
     dev = true,
     minify = false,
   }: {
-    resolutionResponse: ResolutionResponse<Module>,
+    resolutionResponse: ResolutionResponse<Module, T>,
     module: Module,
     name: string,
     map: SourceMap,

packages/metro-bundler/src/Server/index.js

@@ -309,7 +309,7 @@ class Server {
   getDependencies(options: {
     entryFile: string,
     platform: ?string,
-  }): Promise<ResolutionResponse<Module>> {
+  }): Promise<ResolutionResponse<Module, *>> {
     return Promise.resolve().then(() => {
       if (!options.platform) {
         options.platform = getPlatformExtension(options.entryFile);

packages/metro-bundler/src/lib/GlobalTransformCache.js

@@ -11,6 +11,8 @@
 
 'use strict';
 
+/* global Buffer: true */
+
 const BatchProcessor = require('./BatchProcessor');
 const FetchError = require('node-fetch/lib/fetch-error');
 
@@ -380,14 +382,16 @@ class OptionsHasher {
    * particular file.
    */
   hashTransformOptions(hash: crypto$Hash, options: TransformOptions): crypto$Hash {
-    const {generateSourceMaps, dev, hot, inlineRequires, platform,
-      preloadedModules, projectRoots, ramGroups, ...unknowns} = options;
+    const {
+      generateSourceMaps, dev, hot, inlineRequires, platform, projectRoots,
+      ...unknowns,
+    } = options;
     const unknownKeys = Object.keys(unknowns);
     if (unknownKeys.length > 0) {
       const message = `these transform option fields are unknown: ${JSON.stringify(unknownKeys)}`;
       throw new CannotHashOptionsError(message);
     }
-    // eslint-disable-next-line no-undef
+
     hash.update(new Buffer([
       // eslint-disable-next-line no-bitwise
       +dev | +generateSourceMaps << 1 | +hot << 2 | +!!inlineRequires << 3,
@@ -398,7 +402,7 @@ class OptionsHasher {
       relativeBlacklist = this.relativizeFilePaths(Object.keys(inlineRequires.blacklist));
     }
     const relativeProjectRoots = this.relativizeFilePaths(projectRoots);
-    const optionTuple = [relativeBlacklist, preloadedModules, relativeProjectRoots, ramGroups];
+    const optionTuple = [relativeBlacklist, relativeProjectRoots];
     hash.update(JSON.stringify(optionTuple));
     return hash;
   }

packages/metro-bundler/src/lib/__tests__/GlobalTransformCache-test.js

@@ -51,9 +51,7 @@ describe('GlobalTransformCache', () => {
         hot: false,
         inlineRequires: false,
         platform: 'ios',
-        preloadedModules: [],
         projectRoots: [path.join(__dirname, 'root')],
-        ramGroups: [],
       },
     };
     const result = await Promise.all([cache.fetch({

packages/metro-bundler/src/lib/__tests__/__snapshots__/GlobalTransformCache-test.js.snap

@@ -19,12 +19,12 @@ Object {
 exports[`GlobalTransformCache fetches results 1`] = `
 Array [
   Object {
-    "code": "/* code from http://globalcache.com/43e279be6916de8be463ef98bd72ded391608371-foo.js */",
+    "code": "/* code from http://globalcache.com/57c9f999b0df46812db2a6bea665f5feb9d448ff-foo.js */",
     "dependencies": Array [],
     "dependencyOffsets": Array [],
   },
   Object {
-    "code": "/* code from http://globalcache.com/e09598ad4c807cb22b32d68cfc16e0cba7fdb85c-bar.js */",
+    "code": "/* code from http://globalcache.com/df53b0670d117d6b35ce58ff64d09909ece46a09-bar.js */",
     "dependencies": Array [],
     "dependencyOffsets": Array [],
   },

packages/metro-bundler/src/node-haste/DependencyGraph/ResolutionRequest.js

@@ -173,13 +173,13 @@ class ResolutionRequest<TModule: Moduleish, TPackage: Packageish> {
     return [dependencyNames, dependencies];
   }
 
-  getOrderedDependencies({
+  getOrderedDependencies<T>({
     response,
     transformOptions,
     onProgress,
     recursive = true,
   }: {
-    response: ResolutionResponse<TModule>,
+    response: ResolutionResponse<TModule, T>,
     transformOptions: TransformWorkerOptions,
     onProgress?: ?(finishedModules: number, totalModules: number) => mixed,
     recursive: boolean,

packages/metro-bundler/src/node-haste/DependencyGraph/ResolutionResponse.js

@@ -11,18 +11,17 @@
 
 'use strict';
 
-import type {Options as TransformOptions} from '../../JSTransformer/worker/worker';
 import type Module from '../Module';
 
 const NO_OPTIONS = {};
 
-class ResolutionResponse<TModule: {hash(): string}> {
+class ResolutionResponse<TModule: {hash(): string}, TOptions> {
 
-  transformOptions: TransformOptions;
   dependencies: Array<TModule>;
   mainModuleId: ?(number | string);
   mocks: mixed;
   numPrependedDependencies: number;
+  options: TOptions;
 
   // This is monkey-patched from Resolver.
   getModuleId: ?() => number;
@@ -31,12 +30,12 @@ class ResolutionResponse<TModule: {hash(): string}> {
   _finalized: boolean;
   _mainModule: ?TModule;
 
-  constructor({transformOptions}: {transformOptions: TransformOptions}) {
-    this.transformOptions = transformOptions;
+  constructor(options: TOptions) {
     this.dependencies = [];
     this.mainModuleId = null;
     this.mocks = null;
     this.numPrependedDependencies = 0;
+    this.options = options;
     this._mappings = Object.create(null);
     this._finalized = false;
   }
@@ -45,7 +44,7 @@ class ResolutionResponse<TModule: {hash(): string}> {
     dependencies?: Array<TModule>,
     mainModuleId?: number,
     mocks?: mixed,
-  }): ResolutionResponse<TModule> {
+  }): ResolutionResponse<TModule, TOptions> {
     const {
       dependencies = this.dependencies,
       mainModuleId = this.mainModuleId,
@@ -57,7 +56,7 @@ class ResolutionResponse<TModule: {hash(): string}> {
 
     /* $FlowFixMe: Flow doesn't like Object.assign on class-made objects. */
     return Object.assign(
-      new this.constructor({transformOptions: this.transformOptions}),
+      new this.constructor(this.options),
       this,
       {
         dependencies,
@@ -80,7 +79,7 @@ class ResolutionResponse<TModule: {hash(): string}> {
     }
   }
 
-  finalize(): ResolutionResponse<TModule> {
+  finalize(): Promise<this> {
     /* $FlowFixMe: _mainModule is not initialized in the constructor. */
     return this._mainModule.getName().then(id => {
       this.mainModuleId = id;

packages/metro-bundler/src/node-haste/__tests__/DependencyGraph-test.js

@@ -37,10 +37,16 @@ describe('DependencyGraph', function() {
   let Module;
   let ResolutionRequest;
   let defaults;
+  let emptyTransformOptions;
 
   function getOrderedDependenciesAsJSON(dgraphPromise, entryPath, platform, recursive = true) {
     return dgraphPromise
-      .then(dgraph => dgraph.getDependencies({entryPath, platform, recursive}))
+      .then(dgraph => dgraph.getDependencies({
+        entryPath,
+        options: emptyTransformOptions,
+        platform,
+        recursive,
+      }))
       .then(response => response.finalize())
       .then(({dependencies}) => Promise.all(dependencies.map(dep => Promise.all([
         dep.getName(),
@@ -63,6 +69,7 @@ describe('DependencyGraph', function() {
     Module = require('../Module');
     ResolutionRequest = require('../DependencyGraph/ResolutionRequest');
 
+    emptyTransformOptions = {transformer: {transform: {}}};
     defaults = {
       assetExts: ['png', 'jpg'],
       extensions: ['js', 'json'],
@@ -5294,6 +5301,7 @@ describe('DependencyGraph', function() {
       return dependencyGraph.getDependencies({
         entryPath: '/root/index.js',
         onProgress,
+        options: emptyTransformOptions,
       });
     }
 

packages/metro-bundler/src/node-haste/index.js

@@ -37,7 +37,7 @@ const {
 } = require('../Logger');
 const {EventEmitter} = require('events');
 
-import type {Options as TransformOptions} from '../JSTransformer/worker/worker';
+import type {Options as JSTransformerOptions} from '../JSTransformer/worker/worker';
 import type {GlobalTransformCache} from '../lib/GlobalTransformCache';
 import type {GetTransformCacheKey} from '../lib/TransformCache';
 import type {Reporter} from '../lib/reporting';
@@ -179,7 +179,7 @@ class DependencyGraph extends EventEmitter {
    */
   getShallowDependencies(
     entryPath: string,
-    transformOptions: TransformOptions,
+    transformOptions: JSTransformerOptions,
   ): Promise<Array<Module>> {
     return this._moduleCache
       .getModule(entryPath)
@@ -201,19 +201,19 @@ class DependencyGraph extends EventEmitter {
     return Promise.resolve(this._moduleCache.getAllModules());
   }
 
-  getDependencies({
+  getDependencies<T: {+transformer: JSTransformerOptions}>({
     entryPath,
+    options,
     platform,
-    transformOptions,
     onProgress,
     recursive = true,
   }: {
     entryPath: string,
+    options: T,
     platform: ?string,
-    transformOptions: TransformOptions,
     onProgress?: ?(finishedModules: number, totalModules: number) => mixed,
     recursive: boolean,
-  }): Promise<ResolutionResponse<Module>> {
+  }): Promise<ResolutionResponse<Module, T>> {
     platform = this._getRequestPlatform(entryPath, platform);
     const absPath = this._getAbsolutePath(entryPath);
     const dirExists = filePath => {
@@ -236,11 +236,11 @@ class DependencyGraph extends EventEmitter {
       preferNativePlatform: this._opts.preferNativePlatform,
     });
 
-    const response = new ResolutionResponse({transformOptions});
+    const response = new ResolutionResponse(options);
 
     return req.getOrderedDependencies({
       response,
-      transformOptions,
+      transformOptions: options.transformer,
       onProgress,
       recursive,
     }).then(() => response);