Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
[Fix] sync/async Do not leak Object.prototype when checking for c…
…ore modules

[New] add `is-core` export
  • Loading branch information
nicolo-ribaudo authored and ljharb committed Nov 25, 2019
commit c448d087e8057849f15e88567f8555d3a54ef471
5 changes: 2 additions & 3 deletions index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
var core = require('./lib/core');
var async = require('./lib/async');
async.core = core;
async.isCore = function isCore(x) { return core[x]; };
async.core = require('./lib/core');
async.isCore = require('./lib/is-core');
async.sync = require('./lib/sync');

exports = async;
Expand Down
4 changes: 2 additions & 2 deletions lib/async.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
var core = require('./core');
var fs = require('fs');
var path = require('path');
var caller = require('./caller.js');
var nodeModulesPaths = require('./node-modules-paths.js');
var normalizeOptions = require('./normalize-options.js');
var isCore = require('./is-core');

var defaultIsFile = function isFile(file, cb) {
fs.stat(file, function (err, stat) {
Expand Down Expand Up @@ -98,7 +98,7 @@ module.exports = function resolve(x, options, callback) {
} else loadAsFile(res, opts.package, onfile);
} else loadNodeModules(x, basedir, function (err, n, pkg) {
if (err) cb(err);
else if (core[x]) return cb(null, x);
else if (isCore(x)) return cb(null, x);
else if (n) {
return maybeUnwrapSymlink(n, opts, function (err, realN) {
if (err) {
Expand Down
5 changes: 5 additions & 0 deletions lib/is-core.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
var core = require('./core');

module.exports = function isCore(x) {
return Object.prototype.hasOwnProperty.call(core, x);
Copy link
Member

Choose a reason for hiding this comment

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

unfortunately this introduced a bug into v1.13.0 (see #231), by changing this from a "truthy" check, to a "presence" check - and the "core" object relies on true/false to differentiate between a core module in some version of node, versus a core module in the current version of node. This was inadvertently fixed when I extracted this logic into https://npmjs.com/is-core-module.

};
6 changes: 3 additions & 3 deletions lib/sync.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
var core = require('./core');
var isCore = require('./is-core');
var fs = require('fs');
var path = require('path');
var caller = require('./caller.js');
Expand Down Expand Up @@ -68,14 +68,14 @@ module.exports = function (x, options) {
if (x === '..' || x.slice(-1) === '/') res += '/';
var m = loadAsFileSync(res) || loadAsDirectorySync(res);
if (m) return maybeUnwrapSymlink(m, opts);
} else if (core[x]) {
} else if (isCore(x)) {
return x;
} else {
var n = loadNodeModulesSync(x, absoluteStart);
if (n) return maybeUnwrapSymlink(n, opts);
}

if (core[x]) return x;
if (isCore(x)) return x;

var err = new Error("Cannot find module '" + x + "' from '" + parent + "'");
err.code = 'MODULE_NOT_FOUND';
Expand Down
3 changes: 3 additions & 0 deletions test/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ test('core modules', function (t) {

st.ok(!resolve.isCore('seq'));
st.ok(!resolve.isCore('../'));

st.ok(!resolve.isCore('toString'));

st.end();
});

Expand Down