Skip to content

Implements useProcessResolution #170

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed

Implements useProcessResolution #170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e011edc
Implements paths=function and useNodeModules=false
arcanis Oct 19, 2018
5f11711
Makes nodeModulesPaths accept the request
arcanis Oct 19, 2018
109f275
Implements useProcessResolution
arcanis Oct 19, 2018
5797755
Adds documentation
arcanis Oct 19, 2018
5655d0a
Merge branch 'master' into pnp-compat
arcanis Oct 25, 2018
c9faab9
Makes the request an option rather than a parameter
Oct 26, 2018
9daf6b4
Renames useNodeModules into skipNodeModules
Oct 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions lib/async.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ var fs = require('fs');
var path = require('path');
var caller = require('./caller.js');
var nodeModulesPaths = require('./node-modules-paths.js');
var useProcessResolution = require('./use-process-resolution.js');

var defaultIsFile = function isFile(file, cb) {
fs.stat(file, function (err, stat) {
Expand Down Expand Up @@ -38,6 +39,10 @@ module.exports = function resolve(x, options, callback) {
});
}

if (opts.useProcessResolution) {
useProcessResolution(opts);
}

var isFile = opts.isFile || defaultIsFile;
var isDirectory = opts.isDirectory || defaultIsDir;
var readFile = opts.readFile || fs.readFile;
Expand Down Expand Up @@ -225,6 +230,7 @@ module.exports = function resolve(x, options, callback) {
}
}
function loadNodeModules(x, start, cb) {
opts.request = x;
processDirs(cb, nodeModulesPaths(start, opts));
}
};
46 changes: 29 additions & 17 deletions lib/node-modules-paths.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,25 +20,37 @@ module.exports = function nodeModulesPaths(start, opts) {
}
}

var prefix = '/';
if ((/^([A-Za-z]:)/).test(absoluteStart)) {
prefix = '';
} else if ((/^\\\\/).test(absoluteStart)) {
prefix = '\\\\';
}
var dirs = [];

if (!opts || !opts.skipNodeModules) {
var prefix = '/';
if ((/^([A-Za-z]:)/).test(absoluteStart)) {
prefix = '';
} else if ((/^\\\\/).test(absoluteStart)) {
prefix = '\\\\';
}

var paths = [absoluteStart];
var parsed = parse(absoluteStart);
while (parsed.dir !== paths[paths.length - 1]) {
paths.push(parsed.dir);
parsed = parse(parsed.dir);
var paths = [absoluteStart];
var parsed = parse(absoluteStart);
while (parsed.dir !== paths[paths.length - 1]) {
paths.push(parsed.dir);
parsed = parse(parsed.dir);
}

dirs = paths.reduce(function (dirs, aPath) {
return dirs.concat(modules.map(function (moduleDir) {
return path.join(prefix, aPath, moduleDir);
}));
}, []);
}

var dirs = paths.reduce(function (dirs, aPath) {
return dirs.concat(modules.map(function (moduleDir) {
return path.join(prefix, aPath, moduleDir);
}));
}, []);
if (opts && opts.paths) {
if (typeof opts.paths === 'function') {
dirs = dirs.concat(opts.paths(absoluteStart, opts));
} else {
dirs = dirs.concat(opts.paths);
}
}

return opts && opts.paths ? dirs.concat(opts.paths) : dirs;
return dirs;
};
7 changes: 7 additions & 0 deletions lib/sync.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ var fs = require('fs');
var path = require('path');
var caller = require('./caller.js');
var nodeModulesPaths = require('./node-modules-paths.js');
var useProcessResolution = require('./use-process-resolution.js');

var defaultIsFile = function isFile(file) {
try {
Expand All @@ -29,6 +30,11 @@ module.exports = function (x, options) {
throw new TypeError('Path must be a string.');
}
var opts = options || {};

if (opts.useProcessResolution) {
useProcessResolution(opts);
}

var isFile = opts.isFile || defaultIsFile;
var isDirectory = opts.isDirectory || defaultIsDir;
var readFileSync = opts.readFileSync || fs.readFileSync;
Expand Down Expand Up @@ -137,6 +143,7 @@ module.exports = function (x, options) {
}

function loadNodeModulesSync(x, start) {
opts.request = x;
var dirs = nodeModulesPaths(start, opts);
for (var i = 0; i < dirs.length; i++) {
var dir = dirs[i];
Expand Down
31 changes: 31 additions & 0 deletions lib/use-process-resolution.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
var path = require('path');

module.exports = function processResolution(opts) {
if (process.versions.pnp) {
var pnp = require('pnpapi');

opts.preserveSymlinks = true;
opts.skipNodeModules = true;
opts.paths = function (basedir, opts) {
if (!opts || !opts.request) {
throw new Error('Missing request option');
}

// Extract the name of the package being requested (1=full name, 2=scope name, 3=local name)
var parts = opts.request.match(/^((?:(@[^/]+)\/)?([^/]+))/);

// This is guaranteed to return the path to the "package.json" file from the given package
var manifestPath = pnp.resolveToUnqualified(parts[1] + '/package.json', basedir);

// The first dirname strips the package.json, the second strips the local named folder
var nodeModules = path.dirname(path.dirname(manifestPath));

// Strips the scope named folder if needed
if (parts[2]) {
nodeModules = path.dirname(nodeModules);
}

return [nodeModules];
};
}
};
8 changes: 8 additions & 0 deletions readme.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,14 @@ options are:
* returns - a relative path that will be joined from the package.json location

* opts.paths - require.paths array to use if nothing is found on the normal `node_modules` recursive walk (probably don't use this)
For advanced users, `paths` can also be a `opts.paths(start, opts)` function
* start - lookup path
* opts - the resolution options
* opts.request - the import specifier being resolved

* opts.useProcessResolution - instructs `resolve` to use the same resolution algorithm than the one used by the current process

* opts.skipNodeModules - instructs `resolve` to ignore any `node_modules` directory when doing the resolution

* opts.moduleDirectory - directory (or directories) in which to recursively look for modules. default: `"node_modules"`

Expand Down
31 changes: 29 additions & 2 deletions test/node-modules-paths.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ var nodeModulesPaths = require('../lib/node-modules-paths');

var verifyDirs = function verifyDirs(t, start, dirs, moduleDirectories, paths) {
var moduleDirs = [].concat(moduleDirectories || 'node_modules');
if (paths) {
for (var k = 0; k < paths.length; ++k) {
moduleDirs.push(path.basename(paths[k]));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems like a behavior change - the test is now using path.basename on all the paths instead of passing them in directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only one test was using the path parameter previously and it was providing straight folder names (['a', 'b']), so it shouldn't change what's being tested

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair, but why is the basename change needed? Was it incorrect for the test to omit it previously (and there weren't enough test cases to demonstrate that)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, missed the comment, my bad! The basename change was needed because the previous testcases were only using paths to contain single-component paths. Now that there's a test that adds an absolute path, we must be careful not to add the full absolute path into the moduleDirs variable.

}
}

var foundModuleDirs = {};
var uniqueDirs = {};
Expand All @@ -20,7 +25,7 @@ var verifyDirs = function verifyDirs(t, start, dirs, moduleDirectories, paths) {
}
t.equal(keys(parsedDirs).length >= start.split(path.sep).length, true, 'there are >= dirs than "start" has');
var foundModuleDirNames = keys(foundModuleDirs);
t.deepEqual(foundModuleDirNames, moduleDirs.concat(paths || []), 'all desired module dirs were found');
t.deepEqual(foundModuleDirNames, moduleDirs, 'all desired module dirs were found');
t.equal(keys(uniqueDirs).length, dirs.length, 'all dirs provided were unique');

var counts = {};
Expand Down Expand Up @@ -49,7 +54,16 @@ test('node-modules-paths', function (t) {
t.end();
});

t.test('with paths option', function (t) {
t.test('with skipNodeModules=true option', function (t) {
var start = path.join(__dirname, 'resolver');
var dirs = nodeModulesPaths(start, { skipNodeModules: true });

t.deepEqual(dirs, [], 'no node_modules was computed');

t.end();
});

t.test('with paths=array option', function (t) {
var start = path.join(__dirname, 'resolver');
var paths = ['a', 'b'];
var dirs = nodeModulesPaths(start, { paths: paths });
Expand All @@ -59,6 +73,19 @@ test('node-modules-paths', function (t) {
t.end();
});

t.test('with paths=function option', function (t) {
var paths = function paths(absoluteStart, opts) {
return [path.join(absoluteStart, 'not node modules', opts.request)];
};

var start = path.join(__dirname, 'resolver');
var dirs = nodeModulesPaths(start, { paths: paths, request: 'pkg' });

verifyDirs(t, start, dirs, null, [path.join(start, 'not node modules', 'pkg')]);

t.end();
});

t.test('with moduleDirectory option', function (t) {
var start = path.join(__dirname, 'resolver');
var moduleDirectory = 'not node modules';
Expand Down