From afe19d8ac57d56c3ccc5ebc05bfd279c83323ce7 Mon Sep 17 00:00:00 2001 From: Jean Lauliac Date: Fri, 12 May 2017 07:31:26 -0700 Subject: [PATCH] packager: never allow platform-specific asset resolution Summary: I stumbled on this while refactoring that function, and i realised that, I believe it doesn't make sense to take into account the platform extension of the "potiential" file path. The reason is, if you try to require "foo.ios.png", the returned asset name would be "foo", and thus we'd try to find "foo.${ext}.png" where `ext` could actually be `android` or anything else! So it's confusing. There's no reason we should allow callsites to specify platform anyway I think. With this changeset we're not losing any functionality, but it might require people to fix some incorrect callsites. Reviewed By: cpojer Differential Revision: D5051791 fbshipit-source-id: 2a1ec7a8bfa6791b6016213305a72bc0b81f23b9 --- packager/src/ModuleGraph/node-haste/node-haste.js | 1 - packager/src/node-haste/DependencyGraph.js | 1 - .../src/node-haste/DependencyGraph/ResolutionRequest.js | 8 +++----- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/packager/src/ModuleGraph/node-haste/node-haste.js b/packager/src/ModuleGraph/node-haste/node-haste.js index ac93bde1c75..38fc879936c 100644 --- a/packager/src/ModuleGraph/node-haste/node-haste.js +++ b/packager/src/ModuleGraph/node-haste/node-haste.js @@ -119,7 +119,6 @@ exports.createResolveFn = function(options: ResolveOptions): ResolveFn { moduleCache, moduleMap: getFakeModuleMap(hasteMap), platform, - platforms, preferNativePlatform: true, sourceExts, }); diff --git a/packager/src/node-haste/DependencyGraph.js b/packager/src/node-haste/DependencyGraph.js index 89e0521873f..bade0b2cb83 100644 --- a/packager/src/node-haste/DependencyGraph.js +++ b/packager/src/node-haste/DependencyGraph.js @@ -233,7 +233,6 @@ class DependencyGraph extends EventEmitter { moduleCache: this._moduleCache, moduleMap: this._moduleMap, platform, - platforms: this._opts.platforms, preferNativePlatform: this._opts.preferNativePlatform, sourceExts: this._opts.sourceExts, }); diff --git a/packager/src/node-haste/DependencyGraph/ResolutionRequest.js b/packager/src/node-haste/DependencyGraph/ResolutionRequest.js index e667cd7c647..7118f545613 100644 --- a/packager/src/node-haste/DependencyGraph/ResolutionRequest.js +++ b/packager/src/node-haste/DependencyGraph/ResolutionRequest.js @@ -88,7 +88,6 @@ type Options = {| +moduleCache: ModuleishCache, +moduleMap: ModuleMap, +platform: ?string, - +platforms: Set, +preferNativePlatform: boolean, +sourceExts: Array, |}; @@ -109,6 +108,8 @@ function tryResolveSync(action: () => T, secondaryAction: () => T): T { } } +const EMPTY_SET = new Set(); + class ResolutionRequest { _doesFileExist = filePath => this._options.hasteFS.exists(filePath); _immediateResolutionCache: {[key: string]: TModule}; @@ -613,10 +614,7 @@ class ResolutionRequest { fromModule: TModule, toModule: string, ): TModule { - const {name, type} = getAssetDataFromName( - potentialModulePath, - this._options.platforms, - ); + const {name, type} = getAssetDataFromName(potentialModulePath, EMPTY_SET); let pattern = '^' + name + '(@[\\d\\.]+x)?'; if (this._options.platform != null) {