module: fix require('.') #1185
module: fix require('.') #1185
Conversation
|
You would be better to add a test about the change. following tests are helpful. https://github.com/iojs/io.js/blob/v1.x/test/parallel/test-require-extensions-main.js |
|
LGTM. While modules are locked, this seems like a bug fix, not a drastic change in existing behavior. It's exceedingly unlikely that anyone is depending on being able to require a file named |
|
Would this be semver-minor or semver-patch? It looks like a new feature to me. |
|
good question, I'd probably lean toward semver-minor since we've become so used to |
|
I'd say semver-patch, since it's a bugfix. It doesn't add a new API or any new docs, it just makes an existing API deliver perform closer to user expectation – I'd venture that no user would reasonably expect |
|
I will add a test tomorrow |
|
I too think a |
targos
force-pushed
the
fix-require
branch
2 times, most recently
from
dec5944
to
6b4cd81
Compare
|
test added |
|
LGTM |
| @@ -205,7 +205,7 @@ Module._resolveLookupPaths = function(request, parent) { | |||
| } | |||
|
|
|||
| var start = request.substring(0, 2); | |||
| if (start !== './' && start !== '..') { | |||
| if (request !== '.' && start !== './' && start !== '..') { | |||
There was a problem hiding this comment.
I don't see why this change is necessary. It could also have been written like this to eliminate the substring call:
if (request[0] !== '.' || (request[1] !== '/' && request[1] !== '.'))But this isn't a hot path, so it does probably not matter.
There was a problem hiding this comment.
The current one looks more readable to me, but feel free to follow up with a change if the perf difference is significant.
|
LGTM I'll try a merge ;) |
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>
|
Merged in 6fc5e95, thanks! |
|
awesome 👍 |
Notable Changes: * Windows: The ongoing work in improving the state of Windows support has resulted in full test suite passes once again. As noted in the release notes for v1.4.2, CI system and configuration problems prevented it from properly reporting problems with the Windows tests, the problems with the CI and the codebase appear to have been fully resolved. * FreeBSD: A kernel bug impacting io.js/Node.js was discovered and a patch has been introduced to prevent it causing problems for io.js (Fedor Indutny) #1218. * module: you can now require('.') instead of having to require('./'), this is considered a bugfix (Michaël Zasso) #1185. * v8: updated to 4.1.0.25 including patches for --max_old_space_size values above 4096 and Solaris support, both of which are already included in io.js.
|
Awesome! 👍 |
fixes #1178