From e6bc8ea10afb7944300f1a32b24d49c1275a1deb Mon Sep 17 00:00:00 2001 From: Jean Lauliac Date: Wed, 17 May 2017 09:21:25 -0700 Subject: [PATCH] packager: MapWithDefaults: @flow Summary: It was hard to type the resolution main algo, I had to put type annotations explicitely everywhere, otherwise Flow would get in some kind of loop and do weird errors. I think it's because the algo is recursive and Flow tries to infer types too deeply because of the generics. Anyway, apart from that it's good to get this extra type security in the few other places. We require Node v4 minimum, that according to the internets supports the `class` syntax without transform, and I verified that inheriting from `Map` actually works as expected. Reviewed By: davidaurelio Differential Revision: D5078023 fbshipit-source-id: 05dfc4acf5b07cdda8a7b36ec9cba216d1810643 --- packager/src/Bundler/index.js | 2 +- packager/src/Server/index.js | 1 + packager/src/lib/ModuleTransport.js | 3 +- .../DependencyGraph/ResolutionRequest.js | 7 ++-- .../DependencyGraph/ResolutionResponse.js | 10 ++--- .../src/node-haste/lib/MapWithDefaults.js | 41 +++++++++++-------- .../lib/__tests__/MapWithDefaults-test.js | 24 +++++++++++ .../node-haste/lib/getInverseDependencies.js | 2 +- 8 files changed, 60 insertions(+), 30 deletions(-) create mode 100644 packager/src/node-haste/lib/__tests__/MapWithDefaults-test.js diff --git a/packager/src/Bundler/index.js b/packager/src/Bundler/index.js index d5da5d7cd14..60edab60380 100644 --- a/packager/src/Bundler/index.js +++ b/packager/src/Bundler/index.js @@ -642,7 +642,7 @@ class Bundler { entryFilePath: string, options: BundlingOptions, getModuleId: () => number, - dependencyPairs: Array<[mixed, {path: string}]>, + dependencyPairs: Array<[string, Module]>, assetPlugins: Array, }): Promise { let moduleTransport; diff --git a/packager/src/Server/index.js b/packager/src/Server/index.js index f9dcf6e626a..fae8323b7c2 100644 --- a/packager/src/Server/index.js +++ b/packager/src/Server/index.js @@ -552,6 +552,7 @@ class Server { changedModules.forEach(m => { response.setResolvedDependencyPairs( m, + /* $FlowFixMe: should be enforced not to be null. */ dependencyPairs.get(m.path), {ignoreFinalized: true}, ); diff --git a/packager/src/lib/ModuleTransport.js b/packager/src/lib/ModuleTransport.js index 1c4e48bdd86..4ba202673ea 100644 --- a/packager/src/lib/ModuleTransport.js +++ b/packager/src/lib/ModuleTransport.js @@ -12,13 +12,14 @@ 'use strict'; import type {RawMapping} from '../Bundler/source-map'; +import type Module from '../node-haste/Module'; import type {SourceMap} from './SourceMap'; type SourceMapOrMappings = SourceMap | Array; type Metadata = { dependencies?: ?Array, - dependencyPairs?: Array<[mixed, {path: string}]>, + dependencyPairs?: Array<[string, Module]>, preloaded: ?boolean, }; diff --git a/packager/src/node-haste/DependencyGraph/ResolutionRequest.js b/packager/src/node-haste/DependencyGraph/ResolutionRequest.js index 4a973778db1..da674da3ba8 100644 --- a/packager/src/node-haste/DependencyGraph/ResolutionRequest.js +++ b/packager/src/node-haste/DependencyGraph/ResolutionRequest.js @@ -214,9 +214,10 @@ class ResolutionRequest { ); }); - const collectedDependencies = new MapWithDefaults(module => - collect(module), - ); + const collectedDependencies: MapWithDefaults< + TModule, + Promise>, + > = new MapWithDefaults(module => collect(module)); const crawlDependencies = (mod, [depNames, dependencies]) => { const filteredPairs = []; diff --git a/packager/src/node-haste/DependencyGraph/ResolutionResponse.js b/packager/src/node-haste/DependencyGraph/ResolutionResponse.js index 2b1bbb134a9..1b3eca62579 100644 --- a/packager/src/node-haste/DependencyGraph/ResolutionResponse.js +++ b/packager/src/node-haste/DependencyGraph/ResolutionResponse.js @@ -11,8 +11,6 @@ 'use strict'; -import type Module from '../Module'; - const NO_OPTIONS = {}; class ResolutionResponse { @@ -26,7 +24,7 @@ class ResolutionResponse { // This is monkey-patched from Resolver. getModuleId: ?() => number; - _mappings: {}; + _mappings: {[hash: string]: Array<[string, TModule]>}; _finalized: boolean; _mainModule: ?TModule; @@ -104,8 +102,8 @@ class ResolutionResponse { } setResolvedDependencyPairs( - module: Module, - pairs: mixed, + module: TModule, + pairs: Array<[string, TModule]>, options: {ignoreFinalized?: boolean} = NO_OPTIONS, ) { if (!options.ignoreFinalized) { @@ -121,7 +119,7 @@ class ResolutionResponse { this.mocks = mocks; } - getResolvedDependencyPairs(module: TModule) { + getResolvedDependencyPairs(module: TModule): $ReadOnlyArray<[string, TModule]> { this._assertFinalized(); return this._mappings[module.hash()]; } diff --git a/packager/src/node-haste/lib/MapWithDefaults.js b/packager/src/node-haste/lib/MapWithDefaults.js index a264f0c2518..ce79e045051 100644 --- a/packager/src/node-haste/lib/MapWithDefaults.js +++ b/packager/src/node-haste/lib/MapWithDefaults.js @@ -1,30 +1,35 @@ - /** +/** * Copyright (c) 2016-present, Facebook, Inc. * All rights reserved. * * This source code is licensed under the BSD-style license found in the * LICENSE file in the root directory of this source tree. An additional grant * of patent rights can be found in the PATENTS file in the same directory. + * + * @flow + * @format */ + 'use strict'; -module.exports = function MapWithDefaults(factory, iterable) { - // This can't be `MapWithDefaults extends Map`, b/c the way babel transforms - // super calls in constructors: Map.call(this, iterable) throws for native - // Map objects in node 4+. - // TODO(davidaurelio) switch to a transform that does not transform classes - // and super calls, and change this into a class +class MapWithDefaults extends Map { + _factory: TK => TV; - const map = iterable ? new Map(iterable) : new Map(); - const {get} = map; - map.get = key => { - if (map.has(key)) { - return get.call(map, key); + constructor(factory: TK => TV, iterable?: Iterable<[TK, TV]>) { + super(iterable); + this._factory = factory; + } + + get(key: TK): TV { + if (this.has(key)) { + /* $FlowFixMe: can never be `undefined` since we tested with `has` + * (except if `TV` includes `void` as subtype, ex. is nullable) */ + return Map.prototype.get.call(this, key); } - - const value = factory(key); - map.set(key, value); + const value = this._factory(key); + this.set(key, value); return value; - }; - return map; -}; + } +} + +module.exports = MapWithDefaults; diff --git a/packager/src/node-haste/lib/__tests__/MapWithDefaults-test.js b/packager/src/node-haste/lib/__tests__/MapWithDefaults-test.js new file mode 100644 index 00000000000..efb122b3d7b --- /dev/null +++ b/packager/src/node-haste/lib/__tests__/MapWithDefaults-test.js @@ -0,0 +1,24 @@ +/** + * Copyright (c) 2015-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @format + */ + +'use strict'; + +jest.disableAutomock(); + +const MapWithDefaults = require('../MapWithDefaults'); + +describe('MapWithDefaults', function() { + it('works', () => { + const map = new MapWithDefaults(() => ['bar']); + map.get('foo').push('baz'); + expect(map.get('foo')).toEqual(['bar', 'baz']); + }); +}); diff --git a/packager/src/node-haste/lib/getInverseDependencies.js b/packager/src/node-haste/lib/getInverseDependencies.js index 6ff90334f87..663208ee760 100644 --- a/packager/src/node-haste/lib/getInverseDependencies.js +++ b/packager/src/node-haste/lib/getInverseDependencies.js @@ -17,7 +17,7 @@ import type ResolutionResponse from '../DependencyGraph/ResolutionResponse'; function resolveModuleRequires( resolutionResponse: ResolutionResponse, module: TModule, -) { +): Array { const pairs = resolutionResponse.getResolvedDependencyPairs(module); return pairs ? pairs.map(([, dependencyModule]) => dependencyModule) : []; }