-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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.
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
require should treat "." and ".." like any other directory names #1178
Comments
I too once expected Here's the current error:
|
I can't think of a good reason so I'm +1 on this unless someone comes up with something, perhaps @isaacs can chime in? |
@smcmurray |
Previously, the minimal argument to require the current directory was require('./'). This commits allows to skip the trailing slash. Fixes: #1178 PR-URL: #1185 Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com> Reviewed-By: Christian Tellnes <christian@tellnes.no> Reviewed-By: Roman Reiss <me@silverwind.io>
Fixed by 6fc5e95 |
I know it's nitpicking, but... Since we're bikeshedding the hell out of it anyway, my suggestion for // old method
/*function isRelative(p) {
start = p.substring(0, 2);
if (start !== '.' && start !== './' && start !== '..') {
return false;
} else {
return true;
}
}*/
// proposed method
function isRelative(p) {
if (p[0] !== '.') return false;
if (p.length === 1) return true; // "."
if (p[1] !== '.') return p[1] === '/'; // "./"
if (p.length === 2) return true; // ".."
return p[2] === '/';
}
/*
* Tests
*/
var assert = require('assert');
assert(isRelative('.'));
assert(isRelative('./foo'));
assert(isRelative('..'));
assert(isRelative('../foo'));
assert(!isRelative('.foo'));
assert(!isRelative('..foo')); |
Good suggestion. I (and whoever wrote those module parts) didn't even think about module names starting with dots. I wonder if these are valid npm module names (e.g. they can be published). |
They are not valid module names, npm forbids anything starting with a dot, see validate-npm-package. So it's purely for consistency reasons. Right now you can create file named "..foo.js" and |
require(".") should load ./index.js
require("..") should load ../index.js
The text was updated successfully, but these errors were encountered: