Change postprocessing hook to work on ModuleTransport instances

Reviewed By: amnn Differential Revision: D4962283 fbshipit-source-id: 25b609bcd4b8d7a881e35426010f15530548e301
rastaman · Apr 28, 2017 · 414d5a3 · 414d5a3
1 parent 1e6bca2
commit 414d5a3
Show file tree

Hide file tree

Showing 7 changed files with 88 additions and 47 deletions.
diff --git a/local-cli/core/index.js b/local-cli/core/index.js
@@ -15,8 +15,8 @@ const Config = require('../util/Config');
 const defaultConfig = require('./default.config');
 const minimist = require('minimist');
 
-import type {GetTransformOptions} from '../../packager/src/Bundler';
-import type Module, {HasteImpl} from '../../packager/src/node-haste/Module';
+import type {GetTransformOptions, PostProcessModules} from '../../packager/src/Bundler';
+import type {HasteImpl} from '../../packager/src/node-haste/Module';
 import type {CommandT} from '../commands';
 
 /**
@@ -72,7 +72,7 @@ export type ConfigT = {
    * An optional function that can modify the module array before the bundle is
    * finalized.
    */
-  postProcessModules?: (modules: Array<Module>, entryFile: string) => Array<Module>,
+  postProcessModules?: PostProcessModules,
 
   /**
    * A module that exports:

diff --git a/local-cli/server/runServer.js b/local-cli/server/runServer.js
@@ -110,6 +110,7 @@ function getPackagerServer(args, config) {
     getTransformOptions: config.getTransformOptions,
     hasteImpl: config.hasteImpl,
     platforms: defaultPlatforms.concat(args.platforms),
+    postProcessModules: config.postProcessModules,
     projectRoots: args.projectRoots,
     providesModuleNodeModules: providesModuleNodeModules,
     reporter: new LogReporter(),

diff --git a/packager/react-packager.js b/packager/react-packager.js
@@ -17,6 +17,7 @@ const debug = require('debug');
 const invariant = require('fbjs/lib/invariant');
 
 import type Server from './src/Server';
+import type {PostProcessModules} from './src/Bundler';
 import type {GlobalTransformCache} from './src/lib/GlobalTransformCache';
 import type {Reporter} from './src/lib/reporting';
 import type {HasteImpl} from './src/node-haste/Module';
@@ -28,19 +29,13 @@ type Options = {
   hasteImpl?: HasteImpl,
   globalTransformCache: ?GlobalTransformCache,
   nonPersistent?: boolean,
+  postProcessModules?: PostProcessModules,
   projectRoots: Array<string>,
   reporter?: Reporter,
   watch?: boolean,
 };
 
-type StrictOptions = {
-  hasteImpl?: HasteImpl,
-  globalTransformCache: ?GlobalTransformCache,
-  nonPersistent?: boolean,
-  projectRoots: Array<string>,
-  reporter: Reporter,
-  watch?: boolean,
-};
+type StrictOptions = {...Options, reporter: Reporter};
 
 type PublicBundleOptions = {
   +dev?: boolean,

diff --git a/packager/src/Bundler/__tests__/Bundler-test.js b/packager/src/Bundler/__tests__/Bundler-test.js
@@ -34,6 +34,8 @@ var sizeOf = require('image-size');
 var fs = require('fs');
 const os = require('os');
 
+const {any, objectContaining} = expect;
+
 var commonOptions = {
   allowBundleUpdates: false,
   assetExts: defaults.assetExts,
@@ -304,14 +306,34 @@ describe('Bundler', function() {
         assetServer,
       });
 
+      const dev = false;
+      const minify = true;
+      const platform = 'arbitrary';
+
       const entryFile = '/root/foo.js';
       return b.bundle({
+        dev,
         entryFile,
+        minify,
+        platform,
         runBeforeMainModule: [],
         runModule: true,
         sourceMapUrl: 'source_map_url',
       }).then(() => {
-        expect(postProcessModules).toBeCalledWith(modules, entryFile);
+        expect(postProcessModules)
+          .toBeCalledWith(
+            modules.map(x => objectContaining({
+              name: any(String),
+              id: any(Number),
+              code: any(String),
+              sourceCode: any(String),
+              sourcePath: x.path,
+              meta: any(Object),
+              polyfill: !!x.isPolyfill(),
+            })),
+            entryFile,
+            {dev, minify, platform},
+          );
       });
     });
 

diff --git a/packager/src/Bundler/index.js b/packager/src/Bundler/index.js
@@ -100,6 +100,18 @@ const assetPropertyBlacklist = new Set([
   'path',
 ]);
 
+export type PostProcessModulesOptions = {|
+  dev: boolean,
+  minify: boolean,
+  platform: string,
+|};
+
+export type PostProcessModules = (
+  modules: Array<ModuleTransport>,
+  entryFile: string,
+  options: PostProcessModulesOptions,
+) => Array<ModuleTransport>;
+
 type Options = {|
   +allowBundleUpdates: boolean,
   +assetExts: Array<string>,
@@ -112,7 +124,7 @@ type Options = {|
   +hasteImpl?: HasteImpl,
   +platforms: Array<string>,
   +polyfillModuleNames: Array<string>,
-  +postProcessModules?: (modules: Array<Module>, entryFile: string) => Array<Module>,
+  +postProcessModules?: PostProcessModules,
   +projectRoots: Array<string>,
   +providesModuleNodeModules?: Array<string>,
   +reporter: Reporter,
@@ -450,35 +462,44 @@ class Bundler {
         }
       }
 
-      const toModuleTransport = module =>
-        this._toModuleTransport({
-          module,
-          bundle,
-          entryFilePath,
-          assetPlugins,
-          options: response.options,
-          /* $FlowFixMe: `getModuleId` is monkey-patched */
-          getModuleId: (response.getModuleId: () => number),
-          dependencyPairs: response.getResolvedDependencyPairs(module),
-        }).then(transformed => {
-          modulesByName[transformed.name] = module;
-          onModuleTransformed({
+      const modulesByTransport: Map<ModuleTransport, Module> = new Map();
+      const toModuleTransport: Module => Promise<ModuleTransport> =
+        module =>
+          this._toModuleTransport({
             module,
-            response,
             bundle,
-            transformed,
+            entryFilePath,
+            assetPlugins,
+            options: response.options,
+            /* $FlowFixMe: `getModuleId` is monkey-patched */
+            getModuleId: (response.getModuleId: () => number),
+            dependencyPairs: response.getResolvedDependencyPairs(module),
+          }).then(transformed => {
+            modulesByTransport.set(transformed, module);
+            modulesByName[transformed.name] = module;
+            onModuleTransformed({
+              module,
+              response,
+              bundle,
+              transformed,
+            });
+            return transformed;
           });
-          return {module, transformed};
-        });
 
-      const deps = this._opts.postProcessModules == null
-        ? response.dependencies
-        : this._opts.postProcessModules(response.dependencies, entryFile);
+      const p = this._opts.postProcessModules;
+      const postProcess = p
+        ? modules => p(modules, entryFile, {dev, minify, platform})
+        : null;
 
-      return Promise.all(deps.map(toModuleTransport))
-        .then(transformedModules =>
-          finalizeBundle({bundle, transformedModules, response, modulesByName})
-        ).then(() => bundle);
+      return Promise.all(response.dependencies.map(toModuleTransport))
+        .then(postProcess)
+        .then(moduleTransports => {
+          const transformedModules = moduleTransports.map(transformed => ({
+            module: modulesByTransport.get(transformed),
+            transformed,
+          }));
+          return finalizeBundle({bundle, transformedModules, response, modulesByName});
+        }).then(() => bundle);
     });
   }
 
@@ -635,9 +656,10 @@ class Bundler {
       [name, {code, dependencies, dependencyOffsets, map, source}]
     ) => {
       const {preloadedModules} = options;
+      const isPolyfill = module.isPolyfill();
       const preloaded =
         module.path === entryFilePath ||
-        module.isPolyfill() ||
+        isPolyfill ||
         preloadedModules && preloadedModules.hasOwnProperty(module.path);
 
       return new ModuleTransport({
@@ -646,6 +668,7 @@ class Bundler {
         code,
         map,
         meta: {dependencies, dependencyOffsets, preloaded, dependencyPairs},
+        polyfill: isPolyfill,
         sourceCode: source,
         sourcePath: module.path,
       });

diff --git a/packager/src/Server/index.js b/packager/src/Server/index.js
@@ -33,7 +33,7 @@ import type ResolutionResponse from '../node-haste/DependencyGraph/ResolutionRes
 import type Bundle from '../Bundler/Bundle';
 import type HMRBundle from '../Bundler/HMRBundle';
 import type {Reporter} from '../lib/reporting';
-import type {GetTransformOptions} from '../Bundler';
+import type {GetTransformOptions, PostProcessModules} from '../Bundler';
 import type {GlobalTransformCache} from '../lib/GlobalTransformCache';
 import type {SourceMap, Symbolicate} from './symbolicate';
 
@@ -68,7 +68,7 @@ type Options = {
   moduleFormat?: string,
   platforms?: Array<string>,
   polyfillModuleNames?: Array<string>,
-  postProcessModules?: (modules: Array<Module>, entryFile: string) => Array<Module>,
+  postProcessModules?: PostProcessModules,
   projectRoots: Array<string>,
   providesModuleNodeModules?: Array<string>,
   reporter: Reporter,
@@ -140,7 +140,7 @@ class Server {
     moduleFormat: string,
     platforms: Array<string>,
     polyfillModuleNames: Array<string>,
-    postProcessModules?: (modules: Array<Module>, entryFile: string) => Array<Module>,
+    postProcessModules?: PostProcessModules,
     projectRoots: Array<string>,
     providesModuleNodeModules?: Array<string>,
     reporter: Reporter,

diff --git a/packager/src/lib/ModuleTransport.js b/packager/src/lib/ModuleTransport.js
@@ -29,9 +29,9 @@ class ModuleTransport {
   code: string;
   sourceCode: string;
   sourcePath: string;
-  virtual: ?boolean;
+  virtual: boolean;
   meta: ?Metadata;
-  polyfill: ?boolean;
+  polyfill: boolean;
   map: ?SourceMapOrMappings;
 
   constructor(data: {
@@ -40,9 +40,9 @@ class ModuleTransport {
     code: string,
     sourceCode: string,
     sourcePath: string,
-    virtual?: ?boolean,
+    virtual?: boolean,
     meta?: ?Metadata,
-    polyfill?: ?boolean,
+    polyfill?: boolean,
     map?: ?SourceMapOrMappings,
   }) {
     this.name = data.name;
@@ -59,9 +59,9 @@ class ModuleTransport {
     assertExists(data, 'sourcePath');
     this.sourcePath = data.sourcePath;
 
-    this.virtual = data.virtual;
+    this.virtual = !!data.virtual;
     this.meta = data.meta;
-    this.polyfill = data.polyfill;
+    this.polyfill = !!data.polyfill;
     this.map = data.map;
 
     Object.freeze(this);