From 5facc237994012b525248a4209841ffebf04a837 Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Wed, 1 Mar 2017 08:10:44 -0800 Subject: [PATCH] Packager - Fix absolute imports on Windows Summary: Absolute imports on Windows were broken, I'm not 100% sure when this happens but when I tested Exponent on Windows which uses `rn-cli.config.js` with ```js getTransformOptions() { return { reactNativePath: path.resolve('./node_modules/react-native'), reactPath: path.resolve('./node_modules/react'), }; } ``` it seemed to use absolute paths for these modules. I also tested absolute paths in node repl and it does work for absolute paths of different formats. `C:/root/test.js`, `/root/test.js`, `C:\root\test.js` all do resolve properly to the same module. To fix this I resolve the absolute path using `path.resolve` on Windows. Noop on other platforms to avoid the overhead since it's not necessary. **Test plan** - Tested that it fixed the bug I had when running Exponent on Windows. - Updated the absolute path test to use forward slashes since this is what happens in practice when using `getTransformOptions`. We can't test all cases on linux since adding the drive letter au Closes https://github.com/facebook/react-native/pull/12530 Differential Revision: D4634699 Pulled By: jeanlauliac fbshipit-source-id: 0cf6528069b79cba2e0f79f48f5a524d59b7091e --- .../node-haste/DependencyGraph/ResolutionRequest.js | 12 +++++++++++- .../src/node-haste/__tests__/DependencyGraph-test.js | 4 ++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/packager/src/node-haste/DependencyGraph/ResolutionRequest.js b/packager/src/node-haste/DependencyGraph/ResolutionRequest.js index 8afb850c3..b59798e61 100644 --- a/packager/src/node-haste/DependencyGraph/ResolutionRequest.js +++ b/packager/src/node-haste/DependencyGraph/ResolutionRequest.js @@ -278,7 +278,7 @@ class ResolutionRequest { _resolveFileOrDir(fromModule: Module, toModuleName: string) { const potentialModulePath = isAbsolutePath(toModuleName) ? - toModuleName : + resolveWindowsPath(toModuleName) : path.join(path.dirname(fromModule.path), toModuleName); return this._redirectRequire(fromModule, potentialModulePath).then( @@ -509,6 +509,16 @@ function normalizePath(modulePath) { return modulePath.replace(/\/$/, ''); } +// HasteFS stores paths with backslashes on Windows, this ensures the path is +// in the proper format. Will also add drive letter if not present so `/root` will +// resolve to `C:\root`. Noop on other platforms. +function resolveWindowsPath(modulePath) { + if (path.sep !== '\\') { + return modulePath; + } + return path.resolve(modulePath); +} + function resolveKeyWithPromise([key, promise]) { return promise.then(value => [key, value]); } diff --git a/packager/src/node-haste/__tests__/DependencyGraph-test.js b/packager/src/node-haste/__tests__/DependencyGraph-test.js index 86149ac97..a37186250 100644 --- a/packager/src/node-haste/__tests__/DependencyGraph-test.js +++ b/packager/src/node-haste/__tests__/DependencyGraph-test.js @@ -2478,7 +2478,7 @@ describe('DependencyGraph', function() { const root = 'C:\\root'; setMockFileSystem({ 'root': { - 'index.js': 'require("C:\\\\root\\\\apple.js");', + 'index.js': 'require("C:/root/apple.js");', 'apple.js': '', }, }); @@ -2493,7 +2493,7 @@ describe('DependencyGraph', function() { { id: 'C:\\root\\index.js', path: 'C:\\root\\index.js', - dependencies: ['C:\\root\\apple.js'], + dependencies: ['C:/root/apple.js'], isAsset: false, isJSON: false, isPolyfill: false,