Skip to content

Commit

Permalink
[Fix] sync: packageFilter: revert "breaking" change in 9f06a8f / #…
Browse files Browse the repository at this point in the history
…202

Fixes #204.

This was a repeat of an unintentionally breaking bugfix which
caused #157, which was reverted in f5c2a41,
which #202 regressed to cause #204.

This time, I've added lots of comments so I won't accidentally do this again.
  • Loading branch information
ljharb committed Nov 26, 2019
1 parent a86405a commit 07612f7
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 13 deletions.
6 changes: 4 additions & 2 deletions lib/sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ module.exports = function (x, options) {
} catch (jsonErr) {}

if (pkg && opts.packageFilter) {
pkg = opts.packageFilter(pkg, pkgfile, dir);
// v2 will pass pkgfile
pkg = opts.packageFilter(pkg, /*pkgfile,*/ dir); // eslint-disable-line spaced-comment
}

return { pkg: pkg, dir: dir };
Expand All @@ -133,7 +134,8 @@ module.exports = function (x, options) {
} catch (e) {}

if (pkg && opts.packageFilter) {
pkg = opts.packageFilter(pkg, pkgfile, x);
// v2 will pass pkgfile
pkg = opts.packageFilter(pkg, /*pkgfile,*/ x); // eslint-disable-line spaced-comment
}

if (pkg && pkg.main) {
Expand Down
3 changes: 2 additions & 1 deletion readme.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,10 @@ options are:

* opts.isDirectory - function to asynchronously test whether a directory exists

* `opts.packageFilter(pkg, pkgfile)` - transform the parsed package.json contents before looking at the "main" field
* `opts.packageFilter(pkg, pkgfile, dir)` - transform the parsed package.json contents before looking at the "main" field
* pkg - package data
* pkgfile - path to package.json
* dir - directory for package.json

* `opts.pathFilter(pkg, path, relativePath)` - transform a path within a package
* pkg - package data
Expand Down
16 changes: 10 additions & 6 deletions test/filter_sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ test('filter', function (t) {
var packageFilterArgs;
var res = resolve.sync('./baz', {
basedir: dir,
packageFilter: function (pkg, pkgfile, dir) {
// NOTE: in v2.x, this will be `pkg, pkgfile, dir`, but must remain "broken" here in v1.x for compatibility
packageFilter: function (pkg, /*pkgfile,*/ dir) { // eslint-disable-line spaced-comment
pkg.main = 'doom'; // eslint-disable-line no-param-reassign
packageFilterArgs = [pkg, pkgfile, dir];
packageFilterArgs = 'is 1.x' ? [pkg, dir] : [pkg, pkgfile, dir]; // eslint-disable-line no-constant-condition, no-undef
return pkg;
}
});
Expand All @@ -19,11 +20,14 @@ test('filter', function (t) {
var packageData = packageFilterArgs[0];
t.equal(packageData.main, 'doom', 'package "main" was altered');

var packageFile = packageFilterArgs[1];
t.equal(packageFile, path.join(dir, 'baz', 'package.json'), 'package.json path is correct');
if (!'is 1.x') { // eslint-disable-line no-constant-condition
var packageFile = packageFilterArgs[1];
t.equal(packageFile, path.join(dir, 'baz', 'package.json'), 'package.json path is correct');
}

var packageDir = packageFilterArgs[2];
t.equal(packageDir, path.join(dir, 'baz'), 'third packageFilter argument is "dir"');
var packageDir = packageFilterArgs['is 1.x' ? 1 : 2]; // eslint-disable-line no-constant-condition
// eslint-disable-next-line no-constant-condition
t.equal(packageDir, path.join(dir, 'baz'), ('is 1.x' ? 'second' : 'third') + ' packageFilter argument is "dir"');

t.end();
});
14 changes: 10 additions & 4 deletions test/symlinks.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,19 +105,21 @@ test('packageFilter', function (t) {

function testPackageFilter(preserveSymlinks) {
return function (st) {
st.plan(5);
st.plan('is 1.x' ? 3 : 5); // eslint-disable-line no-constant-condition

var destMain = 'symlinks/dest/node_modules/mod-a/index.js';
var destPkg = 'symlinks/dest/node_modules/mod-a/package.json';
var sourceMain = 'symlinks/source/node_modules/mod-a/index.js';
var sourcePkg = 'symlinks/source/node_modules/mod-a/package.json';
var destDir = path.join(__dirname, 'symlinks', 'dest');

/* eslint multiline-comment-style: 0 */
/* v2.x will restore these tests
var packageFilterPath = [];
var actualPath = resolve.sync('mod-a', {
basedir: destDir,
preserveSymlinks: preserveSymlinks,
packageFilter: function (pkg, pkgfile) {
packageFilter: function (pkg, pkgfile, dir) {
packageFilterPath.push(pkgfile);
}
});
Expand All @@ -131,6 +133,7 @@ test('packageFilter', function (t) {
map(preserveSymlinks ? [destPkg, destPkg] : [sourcePkg, sourcePkg], path.normalize),
'sync: packageFilter pkgfile arg is correct'
);
*/

var asyncPackageFilterPath = [];
resolve(
Expand All @@ -150,8 +153,11 @@ test('packageFilter', function (t) {
'async: actual path is correct'
);
st.deepEqual(
map(packageFilterPath, relative),
map(preserveSymlinks ? [destPkg, destPkg] : [sourcePkg, sourcePkg], path.normalize),
map(asyncPackageFilterPath, relative),
map(
preserveSymlinks ? [destPkg, destPkg, destPkg] : [sourcePkg, sourcePkg, sourcePkg],
path.normalize
),
'async: packageFilter pkgfile arg is correct'
);
}
Expand Down

0 comments on commit 07612f7

Please sign in to comment.