module: fix resolution of filename with trailing slash#6215
module: fix resolution of filename with trailing slash#6215targos wants to merge 1 commit intonodejs:masterfrom
Conversation
|
I added a test in a separate commit but I'm not sure it's very useful. If the fixtures are removed by accident, the test would still pass. |
|
LGTM |
|
Does this resolve issues with |
Currently in node 6 (RC2) the `require('../')` breaks with the below error:
```
module.js:128
for (var i = 0; i < exts.length; i++) {
^
TypeError: Cannot read property 'length' of undefined
at tryExtensions (module.js:128:27)
at tryPackage (module.js:107:10)
at Function.Module._findPath (module.js:182:18)
at Function.Module._resolveFilename (module.js:434:25)
at Function.Module._load (module.js:384:25)
at Module.require (module.js:464:17)
at require (internal/module.js:16:19)
at Object.<anonymous> (/Users/XXXX/Documents/Personal/Projects/XXXXX/node_modules/mocha/bin/_mocha:12:11)
at Module._compile (module.js:539:32)
at Object.Module._extensions..js (module.js:548:10)
```
Changing to `require('./../lib/mocha')` resolves the issue.
|
LGTM if CI is ok: https://ci.nodejs.org/job/node-test-pull-request/2275/ |
If you want to guard against that, perhaps you can either:
|
|
Is this ready to land? |
|
citgm: https://ci.nodejs.org/job/thealphanerd-smoker/200/ (running because this touches module) |
A recent optimization of module loading performance [1] forgot to check that extensions were set in a certain code path. [1] nodejs@ae18bbe Fixes: nodejs#6214
|
I consolidated the test. |
|
CI is green but there are a couple of CITGM failures. It's not clear if they're related but I doubt it. The failure here looks like it's related to the recent realpath changes. |
|
There has been a flury of activity on master. I'm running citgm on the head of master to see if these breakages are related to something else |
|
So funny enough... without out this fix master is even more broken.... lol |
|
Indeed. if the new CI comes up green I'll be getting this landed. |
|
Landed in 75487f0 |
A recent optimization of module loading performance [1] forgot to check that extensions were set in a certain code path. [1] nodejs@ae18bbe Fixes: nodejs#6214 PR-URL: nodejs#6215 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net>
6 participants
Checklist
Affected core subsystem(s)
module
Description of change
A recent optimization of module loading performance [1] forgot to check that
extensions were set in a certain code path.
[1] ae18bbe
Fixes: #6214