From 95800d156426d6f458384c53751f55e8191da8e7 Mon Sep 17 00:00:00 2001 From: Amjad Masad Date: Wed, 24 Jun 2015 14:50:00 -0700 Subject: [PATCH] [react-packager] Fix the confused node_modules detection function Summary: @public We have a function that detects whether a give file is to be treated as a node_modules. If so it doesn't have access to the haste module map. There is an exception to this rule which is a few modules that are allowed to do that. Currently thats react-native, react-tools, and parse. The current implementation had a bug where if you had `react-native` (or react-tools etc) in the name before the actual package root then the detection will be off. This fixes the problem by starting from the `lastIndexOf('node_modules')` directory, that way nothing confuses us. Test Plan: ./runJestTests.sh export OSS, patch, run e2e test --- .../__tests__/DependencyGraph-test.js | 59 +++++++++++++++++++ .../DependencyGraph/index.js | 12 ++-- 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js b/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js index dbe7dc3a..09861b52 100644 --- a/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js +++ b/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js @@ -2171,6 +2171,65 @@ describe('DependencyGraph', function() { }); }); + pit('should not be confused by prev occuring whitelisted names', function() { + var root = '/react-tools'; + fs.__setMockFilesystem({ + 'react-tools': { + 'index.js': [ + '/**', + ' * @providesModule index', + ' */', + 'require("shouldWork");', + ].join('\n'), + 'node_modules': { + 'react-tools': { + 'package.json': JSON.stringify({ + name: 'react-tools', + main: 'main.js', + }), + 'main.js': [ + '/**', + ' * @providesModule shouldWork', + ' */', + ].join('\n'), + }, + }, + } + }); + + var dgraph = new DependencyGraph({ + roots: [root], + fileWatcher: fileWatcher, + assetExts: ['png', 'jpg'], + }); + return dgraph.getOrderedDependencies('/react-tools/index.js').then(function(deps) { + expect(deps) + .toEqual([ + { + id: 'index', + path: '/react-tools/index.js', + dependencies: ['shouldWork'], + isAsset: false, + isAsset_DEPRECATED: false, + isJSON: false, + isPolyfill: false, + resolution: undefined, + }, + { + id: 'shouldWork', + path: '/react-tools/node_modules/react-tools/main.js', + dependencies: [], + isAsset: false, + isAsset_DEPRECATED: false, + isJSON: false, + isPolyfill: false, + resolution: undefined, + }, + ]); + }); + }); + + pit('should ignore modules it cant find (assumes own require system)', function() { // For example SourceMap.js implements it's own require system. var root = '/root'; diff --git a/react-packager/src/DependencyResolver/DependencyGraph/index.js b/react-packager/src/DependencyResolver/DependencyGraph/index.js index 63ba7875..9390a10e 100644 --- a/react-packager/src/DependencyResolver/DependencyGraph/index.js +++ b/react-packager/src/DependencyResolver/DependencyGraph/index.js @@ -414,18 +414,20 @@ class DependencyGraph { } _isNodeModulesDir(file) { - const inNodeModules = file.indexOf('/node_modules/') !== -1; + let parts = path.normalize(file).split(path.sep); + const indexOfNodeModules = parts.lastIndexOf('node_modules'); - if (!inNodeModules) { + if (indexOfNodeModules === -1) { return false; } + parts = parts.slice(indexOfNodeModules + 1); + const dirs = this._opts.providesModuleNodeModules; for (let i = 0; i < dirs.length; i++) { - const index = file.indexOf(dirs[i]); - if (index !== -1) { - return file.slice(index).indexOf('/node_modules/') !== -1; + if (parts.indexOf(dirs[i]) > -1) { + return false; } }