Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
16 changes: 15 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ function build_resolve_opts(opts, base) {
return opts;
}

function resolve(id, opts, cb) {
function resolve(id, opts, callback) {

// opts.filename
// opts.paths
Expand All @@ -216,6 +216,20 @@ function resolve(id, opts, cb) {
opts = opts || {};
opts.filename = opts.filename || '';

var cb = function(err, path, pkg) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is this set of nested functions trying to do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. if it's a module name, resolve to it
  2. if it's something in the filesystem, then
    2a) if it's a regular file, resolve to it
    2b) if it's a symlink, resolve to its realpath

fs.stat(path, function(notPath) {
if (notPath) callback(err, path, pkg);
else {
fs.realpath(path, function(notReal, real) {
if (notReal) callback(err, path, pkg);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please follow the code style already present in the document.

else {
callback(err, real, pkg);
}
});
}
});
}

var base = path.dirname(opts.filename);

if (opts.basedir) {
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
"empty.js"
],
"scripts": {
"test": "mocha --reporter list test/*.js"
"test": "mocha --reporter list test/*.js",
"postinstall": "node scripts/setup-symlinks.js"
Copy link
Collaborator

Choose a reason for hiding this comment

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

End users don't need this link to happen so I'd prefer if the module didn't do it for all users.

},
"repository": {
"type": "git",
Expand Down
10 changes: 10 additions & 0 deletions scripts/setup-symlinks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
var fs = require('fs');

try {
fs.mkdirSync(__dirname + '/../test/fixtures/node_modules/linker/node_modules');
} catch (e) {}
process.chdir(__dirname + '/../test/fixtures/node_modules/linker/node_modules');
try {
fs.unlinkSync('linked');
} catch (e) {}
fs.symlinkSync('../../../linked', 'linked', 'dir');
1 change: 1 addition & 0 deletions test/fixtures/linked/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// dummy
1 change: 1 addition & 0 deletions test/fixtures/node_modules/linker/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions test/modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -319,3 +319,25 @@ test('not fail on accessing path name defined in Object.prototype', function (do
done();
});
});

test('respect symlinks', function (done) {
// some systems (e.g. rush, pnpm) use symlinks to create a recursive dependency graph
// instead of relying on the hoisting aspect of the node module resolution algorithm (like npm and yarn do)
// in these cases, we want to resolve to the real path of a module, so that the tree structure below
// only ever tries to run the `x` module once (rather than once on each module that depends on it)
//
// - node_modules
// - a
// - node_modules
// - symlink to x
// - b
// - node_modules
// - symlink to x
//
resolve('linked', { paths: [ fixtures_dir + '/linker/node_modules' ], package: { main: 'fixtures' } }, function(err, path, pkg) {
assert.ifError(err);
assert.equal(path, require.resolve('./fixtures/linked/index'));
assert.strictEqual(pkg, undefined);
done();
});
});