From a1f4dac064623b8fc4a37061257cafb250e37aff Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Fri, 2 Dec 2016 04:43:53 -0800 Subject: [PATCH] Fix packager asset requests on windows Summary: 6554ad5983ae85212684a09869e2ea2e0b743dd3 broke assets on windows, this fixes it and add a test to avoid regressions. Ideally we'd run all the Dependency graph tests on both posix and win32 filesystems but that would probably require doing some sort of factory function to create the tests because I don't think we want to duplicate every test (file is big enough already :)). So for now I just copied that one test and changed the paths manually. **Test plan** Run the new test without the fix -> fails Run the new test with the fix -> succeeds Closes https://github.com/facebook/react-native/pull/11254 Differential Revision: D4265157 Pulled By: cpojer fbshipit-source-id: 511470276bd950c2943e94c2dce6840df0fe6d69 --- .../DependencyGraph/ResolutionRequest.js | 7 +- .../__tests__/DependencyGraph-test.js | 75 +++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/react-packager/src/node-haste/DependencyGraph/ResolutionRequest.js b/react-packager/src/node-haste/DependencyGraph/ResolutionRequest.js index ad683953..e3b5cdfa 100644 --- a/react-packager/src/node-haste/DependencyGraph/ResolutionRequest.js +++ b/react-packager/src/node-haste/DependencyGraph/ResolutionRequest.js @@ -381,7 +381,7 @@ class ResolutionRequest { _loadAsFile(potentialModulePath, fromModule, toModule) { return Promise.resolve().then(() => { if (this._helpers.isAssetFile(potentialModulePath)) { - const dirname = path.dirname(potentialModulePath); + let dirname = path.dirname(potentialModulePath); if (!this._dirExists(dirname)) { throw new UnableToResolveError( fromModule, @@ -398,6 +398,11 @@ class ResolutionRequest { } pattern += '\\.' + type; + // Escape backslashes in the path to be able to use it in the regex + if (path.sep === '\\') { + dirname = dirname.replace(/\\/g, '\\\\'); + } + // We arbitrarly grab the first one, because scale selection // will happen somewhere const [assetFile] = this._hasteFS.matchFiles( diff --git a/react-packager/src/node-haste/__tests__/DependencyGraph-test.js b/react-packager/src/node-haste/__tests__/DependencyGraph-test.js index ecb5c19b..da91d092 100644 --- a/react-packager/src/node-haste/__tests__/DependencyGraph-test.js +++ b/react-packager/src/node-haste/__tests__/DependencyGraph-test.js @@ -2562,6 +2562,81 @@ describe('DependencyGraph', function() { ]); }); }); + + it('should get dependencies with assets and resolution', function() { + const root = 'C:\\root'; + setMockFileSystem({ + 'root': { + 'index.js': [ + '/**', + ' * @providesModule index', + ' */', + 'require("./imgs/a.png");', + 'require("./imgs/b.png");', + 'require("./imgs/c.png");', + ].join('\n'), + 'imgs': { + 'a@1.5x.png': '', + 'b@.7x.png': '', + 'c.png': '', + 'c@2x.png': '', + }, + 'package.json': JSON.stringify({ + name: 'rootPackage', + }), + }, + }); + + var dgraph = new DependencyGraph({ + ...defaults, + roots: [root], + }); + return getOrderedDependenciesAsJSON(dgraph, 'C:\\root\\index.js').then(function(deps) { + expect(deps) + .toEqual([ + { + id: 'index', + path: 'C:\\root\\index.js', + dependencies: [ + './imgs/a.png', + './imgs/b.png', + './imgs/c.png', + ], + isAsset: false, + isJSON: false, + isPolyfill: false, + resolution: undefined, + }, + { + id: 'rootPackage/imgs/a.png', + path: 'C:\\root\\imgs\\a@1.5x.png', + resolution: 1.5, + dependencies: [], + isAsset: true, + isJSON: false, + isPolyfill: false, + }, + { + id: 'rootPackage/imgs/b.png', + path: 'C:\\root\\imgs\\b@.7x.png', + resolution: 0.7, + dependencies: [], + isAsset: true, + isJSON: false, + isPolyfill: false, + }, + { + id: 'rootPackage/imgs/c.png', + path: 'C:\\root\\imgs\\c.png', + resolution: 1, + dependencies: [], + isAsset: true, + isJSON: false, + isPolyfill: false, + }, + ]); + }); + }); }); describe('node_modules (posix)', function() {