Skip to content

Commit d77b4fd

Browse files
janicduplessisMartin Konicek
authored and
Martin Konicek
committed
Fix packager asset requests on windows
Summary: 6554ad5 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 #11254 Differential Revision: D4265157 Pulled By: cpojer fbshipit-source-id: 511470276bd950c2943e94c2dce6840df0fe6d69
1 parent ca403f0 commit d77b4fd

File tree

2 files changed

+81
-1
lines changed

2 files changed

+81
-1
lines changed

packager/react-packager/src/node-haste/DependencyGraph/ResolutionRequest.js

+6-1
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,7 @@ class ResolutionRequest {
381381
_loadAsFile(potentialModulePath, fromModule, toModule) {
382382
return Promise.resolve().then(() => {
383383
if (this._helpers.isAssetFile(potentialModulePath)) {
384-
const dirname = path.dirname(potentialModulePath);
384+
let dirname = path.dirname(potentialModulePath);
385385
if (!this._dirExists(dirname)) {
386386
throw new UnableToResolveError(
387387
fromModule,
@@ -398,6 +398,11 @@ class ResolutionRequest {
398398
}
399399
pattern += '\\.' + type;
400400

401+
// Escape backslashes in the path to be able to use it in the regex
402+
if (path.sep === '\\') {
403+
dirname = dirname.replace(/\\/g, '\\\\');
404+
}
405+
401406
// We arbitrarly grab the first one, because scale selection
402407
// will happen somewhere
403408
const [assetFile] = this._hasteFS.matchFiles(

packager/react-packager/src/node-haste/__tests__/DependencyGraph-test.js

+75
Original file line numberDiff line numberDiff line change
@@ -2562,6 +2562,81 @@ describe('DependencyGraph', function() {
25622562
]);
25632563
});
25642564
});
2565+
2566+
it('should get dependencies with assets and resolution', function() {
2567+
const root = 'C:\\root';
2568+
setMockFileSystem({
2569+
'root': {
2570+
'index.js': [
2571+
'/**',
2572+
' * @providesModule index',
2573+
' */',
2574+
'require("./imgs/a.png");',
2575+
'require("./imgs/b.png");',
2576+
'require("./imgs/c.png");',
2577+
].join('\n'),
2578+
'imgs': {
2579+
'a@1.5x.png': '',
2580+
'b@.7x.png': '',
2581+
'c.png': '',
2582+
'c@2x.png': '',
2583+
},
2584+
'package.json': JSON.stringify({
2585+
name: 'rootPackage',
2586+
}),
2587+
},
2588+
});
2589+
2590+
var dgraph = new DependencyGraph({
2591+
...defaults,
2592+
roots: [root],
2593+
});
2594+
return getOrderedDependenciesAsJSON(dgraph, 'C:\\root\\index.js').then(function(deps) {
2595+
expect(deps)
2596+
.toEqual([
2597+
{
2598+
id: 'index',
2599+
path: 'C:\\root\\index.js',
2600+
dependencies: [
2601+
'./imgs/a.png',
2602+
'./imgs/b.png',
2603+
'./imgs/c.png',
2604+
],
2605+
isAsset: false,
2606+
isJSON: false,
2607+
isPolyfill: false,
2608+
resolution: undefined,
2609+
},
2610+
{
2611+
id: 'rootPackage/imgs/a.png',
2612+
path: 'C:\\root\\imgs\\a@1.5x.png',
2613+
resolution: 1.5,
2614+
dependencies: [],
2615+
isAsset: true,
2616+
isJSON: false,
2617+
isPolyfill: false,
2618+
},
2619+
{
2620+
id: 'rootPackage/imgs/b.png',
2621+
path: 'C:\\root\\imgs\\b@.7x.png',
2622+
resolution: 0.7,
2623+
dependencies: [],
2624+
isAsset: true,
2625+
isJSON: false,
2626+
isPolyfill: false,
2627+
},
2628+
{
2629+
id: 'rootPackage/imgs/c.png',
2630+
path: 'C:\\root\\imgs\\c.png',
2631+
resolution: 1,
2632+
dependencies: [],
2633+
isAsset: true,
2634+
isJSON: false,
2635+
isPolyfill: false,
2636+
},
2637+
]);
2638+
});
2639+
});
25652640
});
25662641

25672642
describe('node_modules (posix)', function() {

0 commit comments

Comments
 (0)