Skip to content

Commit 13a765b

Browse files
ljharbljharb
committed
[Fix] sync: packageFilter: when preserveSymlinks is false, realpath the pkgfile option
 - `async`/`sync`: avoid crash when `package.json` does not exist, or `packageFilter` returns a falsy value Fixes #201.
1 parent 7628fe2 commit 13a765b

File tree

6 files changed

+139
-51
lines changed

6 files changed

+139
-51
lines changed

‎lib/async.js

Lines changed: 49 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -171,19 +171,21 @@ module.exports = function resolve(x, options, callback) {
171171
}
172172
if ((/[/\\]node_modules[/\\]*$/).test(dir)) return cb(null);
173173

174-
var pkgfile = path.join(dir, 'package.json');
175-
isFile(pkgfile, function (err, ex) {
176-
// on err, ex is false
177-
if (!ex) return loadpkg(path.dirname(dir), cb);
178-
179-
readFile(pkgfile, function (err, body) {
180-
if (err) cb(err);
181-
try { var pkg = JSON.parse(body); } catch (jsonErr) {}
182-
183-
if (pkg && opts.packageFilter) {
184-
pkg = opts.packageFilter(pkg, pkgfile);
185-
}
186-
cb(null, pkg, dir);
174+
maybeUnwrapSymlink(path.join(dir, 'package.json'), opts, function (unwrapErr, pkgfile) {
175+
if (unwrapErr) return loadpkg(path.dirname(dir), cb);
176+
isFile(pkgfile, function (err, ex) {
177+
// on err, ex is false
178+
if (!ex) return loadpkg(path.dirname(dir), cb);
179+
180+
readFile(pkgfile, function (err, body) {
181+
if (err) cb(err);
182+
try { var pkg = JSON.parse(body); } catch (jsonErr) {}
183+
184+
if (pkg && opts.packageFilter) {
185+
pkg = opts.packageFilter(pkg, pkgfile);
186+
}
187+
cb(null, pkg, dir);
188+
});
187189
});
188190
});
189191
}
@@ -196,46 +198,48 @@ module.exports = function resolve(x, options, callback) {
196198
fpkg = opts.package;
197199
}
198200

199-
var pkgfile = path.join(x, 'package.json');
200-
isFile(pkgfile, function (err, ex) {
201-
if (err) return cb(err);
202-
if (!ex) return loadAsFile(path.join(x, 'index'), fpkg, cb);
203-
204-
readFile(pkgfile, function (err, body) {
201+
maybeUnwrapSymlink(path.join(x, 'package.json'), opts, function (unwrapErr, pkgfile) {
202+
if (unwrapErr) return cb(unwrapErr);
203+
isFile(pkgfile, function (err, ex) {
205204
if (err) return cb(err);
206-
try {
207-
var pkg = JSON.parse(body);
208-
} catch (jsonErr) {}
205+
if (!ex) return loadAsFile(path.join(x, 'index'), fpkg, cb);
209206

210-
if (opts.packageFilter) {
211-
pkg = opts.packageFilter(pkg, pkgfile);
212-
}
207+
readFile(pkgfile, function (err, body) {
208+
if (err) return cb(err);
209+
try {
210+
var pkg = JSON.parse(body);
211+
} catch (jsonErr) {}
213212

214-
if (pkg.main) {
215-
if (typeof pkg.main !== 'string') {
216-
var mainError = new TypeError('package “' + pkg.name + '” `main` must be a string');
217-
mainError.code = 'INVALID_PACKAGE_MAIN';
218-
return cb(mainError);
219-
}
220-
if (pkg.main === '.' || pkg.main === './') {
221-
pkg.main = 'index';
213+
if (pkg && opts.packageFilter) {
214+
pkg = opts.packageFilter(pkg, pkgfile);
222215
}
223-
loadAsFile(path.resolve(x, pkg.main), pkg, function (err, m, pkg) {
224-
if (err) return cb(err);
225-
if (m) return cb(null, m, pkg);
226-
if (!pkg) return loadAsFile(path.join(x, 'index'), pkg, cb);
227216

228-
var dir = path.resolve(x, pkg.main);
229-
loadAsDirectory(dir, pkg, function (err, n, pkg) {
217+
if (pkg && pkg.main) {
218+
if (typeof pkg.main !== 'string') {
219+
var mainError = new TypeError('package “' + pkg.name + '” `main` must be a string');
220+
mainError.code = 'INVALID_PACKAGE_MAIN';
221+
return cb(mainError);
222+
}
223+
if (pkg.main === '.' || pkg.main === './') {
224+
pkg.main = 'index';
225+
}
226+
loadAsFile(path.resolve(x, pkg.main), pkg, function (err, m, pkg) {
230227
if (err) return cb(err);
231-
if (n) return cb(null, n, pkg);
232-
loadAsFile(path.join(x, 'index'), pkg, cb);
228+
if (m) return cb(null, m, pkg);
229+
if (!pkg) return loadAsFile(path.join(x, 'index'), pkg, cb);
230+
231+
var dir = path.resolve(x, pkg.main);
232+
loadAsDirectory(dir, pkg, function (err, n, pkg) {
233+
if (err) return cb(err);
234+
if (n) return cb(null, n, pkg);
235+
loadAsFile(path.join(x, 'index'), pkg, cb);
236+
});
233237
});
234-
});
235-
return;
236-
}
238+
return;
239+
}
237240

238-
loadAsFile(path.join(x, '/index'), pkg, cb);
241+
loadAsFile(path.join(x, '/index'), pkg, cb);
242+
});
239243
});
240244
});
241245
}

‎lib/sync.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ module.exports = function (x, options) {
105105
}
106106
if ((/[/\\]node_modules[/\\]*$/).test(dir)) return;
107107

108-
var pkgfile = path.join(dir, 'package.json');
108+
var pkgfile = maybeUnwrapSymlink(path.join(dir, 'package.json'), opts);
109109

110110
if (!isFile(pkgfile)) {
111111
return loadpkg(path.dirname(dir));
@@ -118,25 +118,25 @@ module.exports = function (x, options) {
118118
} catch (jsonErr) {}
119119

120120
if (pkg && opts.packageFilter) {
121-
pkg = opts.packageFilter(pkg, dir);
121+
pkg = opts.packageFilter(pkg, pkgfile);
122122
}
123123

124124
return { pkg: pkg, dir: dir };
125125
}
126126

127127
function loadAsDirectorySync(x) {
128-
var pkgfile = path.join(x, '/package.json');
128+
var pkgfile = maybeUnwrapSymlink(path.join(x, '/package.json'), opts);
129129
if (isFile(pkgfile)) {
130130
try {
131131
var body = readFileSync(pkgfile, 'UTF8');
132132
var pkg = JSON.parse(body);
133133
} catch (e) {}
134134

135-
if (opts.packageFilter) {
135+
if (pkg && opts.packageFilter) {
136136
pkg = opts.packageFilter(pkg, x);
137137
}
138138

139-
if (pkg.main) {
139+
if (pkg && pkg.main) {
140140
if (typeof pkg.main !== 'string') {
141141
var mainError = new TypeError('package “' + pkg.name + '” `main` must be a string');
142142
mainError.code = 'INVALID_PACKAGE_MAIN';

‎package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
},
2626
"devDependencies": {
2727
"@ljharb/eslint-config": "^15.0.2",
28+
"array.prototype.map": "^1.0.1",
2829
"eslint": "^6.6.0",
2930
"object-keys": "^1.1.1",
3031
"safe-publish-latest": "^1.1.4",

‎test/symlinks.js

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,26 @@
11
var path = require('path');
22
var fs = require('fs');
33
var test = require('tape');
4+
var map = require('array.prototype.map');
45
var resolve = require('../');
56

67
var symlinkDir = path.join(__dirname, 'resolver', 'symlinked', 'symlink');
78
var packageDir = path.join(__dirname, 'resolver', 'symlinked', '_', 'node_modules', 'package');
9+
var modADir = path.join(__dirname, 'symlinks', 'source', 'node_modules', 'mod-a');
10+
var symlinkModADir = path.join(__dirname, 'symlinks', 'dest', 'node_modules', 'mod-a');
811
try {
912
fs.unlinkSync(symlinkDir);
1013
} catch (err) {}
1114
try {
1215
fs.unlinkSync(packageDir);
1316
} catch (err) {}
17+
try {
18+
fs.unlinkSync(modADir);
19+
} catch (err) {}
20+
try {
21+
fs.unlinkSync(symlinkModADir);
22+
} catch (err) {}
23+
1424
try {
1525
fs.symlinkSync('./_/symlink_target', symlinkDir, 'dir');
1626
} catch (err) {
@@ -23,6 +33,12 @@ try {
2333
// if fails then it is probably on Windows and lets try to create a junction
2434
fs.symlinkSync(path.join(__dirname, '..', '..', 'package') + '\\', packageDir, 'junction');
2535
}
36+
try {
37+
fs.symlinkSync('../../source/node_modules/mod-a', symlinkModADir, 'dir');
38+
} catch (err) {
39+
// if fails then it is probably on Windows and lets try to create a junction
40+
fs.symlinkSync(path.join(__dirname, '..', '..', 'source', 'node_modules', 'mod-a') + '\\', symlinkModADir, 'junction');
41+
}
2642

2743
test('symlink', function (t) {
2844
t.plan(2);
@@ -79,6 +95,69 @@ test('async symlink from node_modules to other dir when preserveSymlinks = false
7995
resolve('package', { basedir: basedir, preserveSymlinks: false }, function (err, result) {
8096
t.notOk(err, 'no error');
8197
t.equal(result, path.resolve(__dirname, 'resolver/symlinked/package/bar.js'));
82-
t.end();
8398
});
8499
});
100+
101+
test('packageFilter', function (t) {
102+
function testPackageFilter(preserveSymlinks) {
103+
return function (st) {
104+
st.plan(5);
105+
106+
var destMain = 'symlinks/dest/node_modules/mod-a/index.js';
107+
var destPkg = 'symlinks/dest/node_modules/mod-a/package.json';
108+
var sourceMain = 'symlinks/source/node_modules/mod-a/index.js';
109+
var sourcePkg = 'symlinks/source/node_modules/mod-a/package.json';
110+
var destDir = path.join(__dirname, 'symlinks', 'dest');
111+
112+
var packageFilterPath = [];
113+
var actualPath = resolve.sync('mod-a', {
114+
basedir: destDir,
115+
preserveSymlinks: preserveSymlinks,
116+
packageFilter: function (pkg, pkgfile) {
117+
packageFilterPath.push(pkgfile);
118+
}
119+
});
120+
st.equal(
121+
actualPath.replace(__dirname + '/', ''),
122+
preserveSymlinks ? destMain : sourceMain,
123+
'sync: actual path is correct'
124+
);
125+
st.deepEqual(
126+
map(packageFilterPath, function (x) { return x.replace(__dirname + '/', ''); }),
127+
preserveSymlinks ? [destPkg, destPkg] : [sourcePkg, sourcePkg],
128+
'sync: packageFilter pkgfile arg is correct'
129+
);
130+
131+
var asyncPackageFilterPath = [];
132+
resolve(
133+
'mod-a',
134+
{
135+
basedir: destDir,
136+
preserveSymlinks: preserveSymlinks,
137+
packageFilter: function (pkg, pkgfile) {
138+
asyncPackageFilterPath.push(pkgfile);
139+
}
140+
},
141+
function (err, actualPath) {
142+
st.error(err, 'no error');
143+
st.equal(
144+
actualPath.replace(__dirname + '/', ''),
145+
preserveSymlinks ? destMain : sourceMain,
146+
'async: actual path is correct'
147+
);
148+
st.deepEqual(
149+
map(asyncPackageFilterPath, function (x) { return x.replace(__dirname + '/', ''); }),
150+
preserveSymlinks ? [destPkg, destPkg, destPkg] : [sourcePkg, sourcePkg, sourcePkg],
151+
'async: packageFilter pkgfile arg is correct'
152+
);
153+
}
154+
);
155+
};
156+
}
157+
158+
t.test('preserveSymlinks: false', testPackageFilter(false));
159+
160+
t.test('preserveSymlinks: true', testPackageFilter(true));
161+
162+
t.end();
163+
});

‎test/symlinks/dest/node_modules/mod-a

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/symlinks/source/node_modules/mod-a/index.js

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)