Skip to content

Commit 30fc738

Browse files
author
Amjad Masad
committed
[react-packager] Fix more node_modules resolution rules
Summary: @public Fixes #773 This fixes `.json` name resolution. And also reads `package.json` when doing a directory module resolution. The algorithm can be found here: https://nodejs.org/api/modules.html I'll probably start including the node (or browserify) modules test in later diffs to make sure we're fully compliant. Test Plan: * ./runJestTests.sh * ./runJestTests.sh PackagerIntegration * open playground and require a json file * test redbox
1 parent 53b2c39 commit 30fc738

File tree

2 files changed

+80
-24
lines changed

2 files changed

+80
-24
lines changed

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

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,11 @@ describe('DependencyGraph', function() {
168168
'/**',
169169
' * @providesModule index',
170170
' */',
171-
'require("./a.json")'
171+
'require("./a.json")',
172+
'require("./b")'
172173
].join('\n'),
173174
'a.json': JSON.stringify({}),
175+
'b.json': JSON.stringify({}),
174176
}
175177
});
176178

@@ -186,7 +188,7 @@ describe('DependencyGraph', function() {
186188
id: 'index',
187189
altId: 'package/index',
188190
path: '/root/index.js',
189-
dependencies: ['./a.json'],
191+
dependencies: ['./a.json', './b'],
190192
isAsset: false,
191193
isAsset_DEPRECATED: false,
192194
isJSON: undefined,
@@ -205,6 +207,17 @@ describe('DependencyGraph', function() {
205207
resolution: undefined,
206208
resolveDependency: undefined,
207209
},
210+
{
211+
id: 'package/b.json',
212+
isJSON: true,
213+
path: '/root/b.json',
214+
dependencies: [],
215+
isAsset: false,
216+
isAsset_DEPRECATED: false,
217+
isPolyfill: false,
218+
resolution: undefined,
219+
resolveDependency: undefined,
220+
},
208221
]);
209222
});
210223
});
@@ -851,6 +864,58 @@ describe('DependencyGraph', function() {
851864
});
852865
});
853866

867+
pit('should resolve require to main if it is a dir w/ a package.json', function() {
868+
var root = '/root';
869+
fs.__setMockFilesystem({
870+
'root': {
871+
'package.json': JSON.stringify({
872+
name: 'test',
873+
}),
874+
'index.js': 'require("./lib/")',
875+
lib: {
876+
'package.json': JSON.stringify({
877+
'main': 'main.js',
878+
}),
879+
'index.js': 'lol',
880+
'main.js': 'lol',
881+
},
882+
}
883+
});
884+
885+
var dgraph = new DependencyGraph({
886+
roots: [root],
887+
fileWatcher: fileWatcher,
888+
assetExts: ['png', 'jpg'],
889+
});
890+
return dgraph.getOrderedDependencies('/root/index.js').then(function(deps) {
891+
expect(getDataFromModules(deps))
892+
.toEqual([
893+
{
894+
id: 'test/index',
895+
path: '/root/index.js',
896+
dependencies: ['./lib/'],
897+
isAsset: false,
898+
isAsset_DEPRECATED: false,
899+
isJSON: undefined,
900+
isPolyfill: false,
901+
resolution: undefined,
902+
resolveDependency: undefined,
903+
},
904+
{
905+
id: '/root/lib/main.js',
906+
path: '/root/lib/main.js',
907+
dependencies: [],
908+
isAsset: false,
909+
isAsset_DEPRECATED: false,
910+
isJSON: undefined,
911+
isPolyfill: false,
912+
resolution: undefined,
913+
resolveDependency: undefined,
914+
},
915+
]);
916+
});
917+
});
918+
854919
pit('should ignore malformed packages', function() {
855920
var root = '/root';
856921
fs.__setMockFilesystem({

packager/react-packager/src/DependencyResolver/haste/DependencyGraph/index.js

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
// TODO
2-
// Fix it to work with tests
3-
41
/**
52
* Copyright (c) 2015-present, Facebook, Inc.
63
* All rights reserved.
@@ -296,16 +293,18 @@ DependecyGraph.prototype.resolveDependency = function(
296293
modulePath = path.join(dir, depModuleId);
297294
modulePath = browserFieldRedirect(packageJson, modulePath);
298295

299-
// JS modules can be required without extensios.
300-
if (!this._isFileAsset(modulePath) && !modulePath.match(/\.json$/)) {
301-
modulePath = withExtJs(modulePath);
302-
}
303-
304-
dep = this._graph[modulePath];
296+
dep = this._graph[modulePath] ||
297+
this._graph[modulePath + '.js'] ||
298+
this._graph[modulePath + '.json'];
305299

306-
// Maybe the dependency is a directory and there is an index.js inside it.
300+
// Maybe the dependency is a directory and there is a packageJson and/or index.js inside it.
307301
if (dep == null) {
308-
dep = this._graph[path.join(dir, depModuleId, 'index.js')];
302+
var dirPackageJson = this._packageByRoot[path.join(dir, depModuleId).replace(/\/$/, '')];
303+
if (dirPackageJson) {
304+
dep = this._resolvePackageMain(dirPackageJson);
305+
} else {
306+
dep = this._graph[path.join(dir, depModuleId, 'index.js')];
307+
}
309308
}
310309

311310
// Maybe it's an asset with @n.nx resolution and the path doesn't map
@@ -444,14 +443,6 @@ DependecyGraph.prototype._processPackage = function(packagePath) {
444443
return Promise.resolve();
445444
}
446445

447-
if (packageJson.name == null) {
448-
debug(
449-
'WARNING: package.json `%s` is missing a name field',
450-
packagePath
451-
);
452-
return Promise.resolve();
453-
}
454-
455446
packageJson._root = packageRoot;
456447
self._addPackageToIndices(packageJson);
457448

@@ -461,14 +452,14 @@ DependecyGraph.prototype._processPackage = function(packagePath) {
461452

462453
DependecyGraph.prototype._addPackageToIndices = function(packageJson) {
463454
this._packageByRoot[packageJson._root] = packageJson;
464-
if (!this._isInNodeModules(packageJson._root)) {
455+
if (!this._isInNodeModules(packageJson._root) && packageJson.name != null) {
465456
this._packagesById[packageJson.name] = packageJson;
466457
}
467458
};
468459

469460
DependecyGraph.prototype._removePackageFromIndices = function(packageJson) {
470461
delete this._packageByRoot[packageJson._root];
471-
if (!this._isInNodeModules(packageJson._root)) {
462+
if (!this._isInNodeModules(packageJson._root) && packageJson.name != null) {
472463
delete this._packagesById[packageJson.name];
473464
}
474465
};
@@ -536,7 +527,7 @@ DependecyGraph.prototype._processModule = function(modulePath) {
536527
*/
537528
DependecyGraph.prototype._lookupName = function(modulePath) {
538529
var packageJson = this._lookupPackage(modulePath);
539-
if (packageJson == null) {
530+
if (packageJson == null || packageJson.name == null) {
540531
return path.resolve(modulePath);
541532
} else {
542533
var relativePath =

0 commit comments

Comments
 (0)