From 414d5a30234f66225786975d35249535efb8de16 Mon Sep 17 00:00:00 2001 From: David Aurelio Date: Fri, 28 Apr 2017 09:27:19 -0700 Subject: [PATCH] Change postprocessing hook to work on `ModuleTransport` instances Reviewed By: amnn Differential Revision: D4962283 fbshipit-source-id: 25b609bcd4b8d7a881e35426010f15530548e301 --- local-cli/core/index.js | 6 +- local-cli/server/runServer.js | 1 + packager/react-packager.js | 11 +-- .../src/Bundler/__tests__/Bundler-test.js | 24 +++++- packager/src/Bundler/index.js | 75 ++++++++++++------- packager/src/Server/index.js | 6 +- packager/src/lib/ModuleTransport.js | 12 +-- 7 files changed, 88 insertions(+), 47 deletions(-) diff --git a/local-cli/core/index.js b/local-cli/core/index.js index 9396de3ec32d8f..a03a1d83cde54d 100644 --- 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, entryFile: string) => Array, + postProcessModules?: PostProcessModules, /** * A module that exports: diff --git a/local-cli/server/runServer.js b/local-cli/server/runServer.js index a9cccbf35da51d..e0163a1ce737f8 100644 --- 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 index b66418c64c6b97..3eecf40f87fd26 100644 --- 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, reporter?: Reporter, watch?: boolean, }; -type StrictOptions = { - hasteImpl?: HasteImpl, - globalTransformCache: ?GlobalTransformCache, - nonPersistent?: boolean, - projectRoots: Array, - 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 index 680ea8f31ae32f..981fd02330ddc5 100644 --- 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 index 6ea0635c023e89..7094c5ff338849 100644 --- 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, + entryFile: string, + options: PostProcessModulesOptions, +) => Array; + type Options = {| +allowBundleUpdates: boolean, +assetExts: Array, @@ -112,7 +124,7 @@ type Options = {| +hasteImpl?: HasteImpl, +platforms: Array, +polyfillModuleNames: Array, - +postProcessModules?: (modules: Array, entryFile: string) => Array, + +postProcessModules?: PostProcessModules, +projectRoots: Array, +providesModuleNodeModules?: Array, +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 = new Map(); + const toModuleTransport: Module => Promise = + 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 index 82c697d4399d6b..17c4ea48325985 100644 --- 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, polyfillModuleNames?: Array, - postProcessModules?: (modules: Array, entryFile: string) => Array, + postProcessModules?: PostProcessModules, projectRoots: Array, providesModuleNodeModules?: Array, reporter: Reporter, @@ -140,7 +140,7 @@ class Server { moduleFormat: string, platforms: Array, polyfillModuleNames: Array, - postProcessModules?: (modules: Array, entryFile: string) => Array, + postProcessModules?: PostProcessModules, projectRoots: Array, providesModuleNodeModules?: Array, reporter: Reporter, diff --git a/packager/src/lib/ModuleTransport.js b/packager/src/lib/ModuleTransport.js index 6f2fd80102e7b7..840de0e959501b 100644 --- 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);