module: fix require in node repl#30835
Uh oh!
There was an error while loading. Please reload this page.
Conversation
ZYSzys added module 
This comment has been minimized.
This comment has been minimized.
| // from realpath(__filename) but with eval there is no filename | ||
| constmainPaths=['.'].concat(Module._nodeModulePaths('.'),modulePaths); | ||
| // from realpath(__filename) but in REPL there is no filename | ||
| constmainPaths=['.']; |
There was a problem hiding this comment.
With --eval, parent has both id and filename set now, so I think this edge case wasn't for --eval anymore but for REPL mode.
> node --eval 'console.log(module)' Module{id: '[eval]', path: '.', exports:{}, parent: undefined, filename: '/Users/zyszys/Projects/nodejs/node/[eval]', loaded: false, children: [], paths: [ '/Users/zyszys/Projects/nodejs/node/node_modules', '/Users/zyszys/Projects/nodejs/node_modules', '/Users/zyszys/Projects/node_modules', '/Users/zyszys/node_modules', '/Users/node_modules', '/node_modules' ] } This comment has been minimized.
This comment has been minimized.
nodejs-github-bot commented Dec 7, 2019
BridgeAR left a comment
BridgeAR left a comment There was a problem hiding this comment.
This does seem correct. It would still be good to get another review from e.g. @devsnek or @guybedford though.
There was a problem hiding this comment.
Can you confirm that node --eval 'require("pkg")' where the file system looks like:
/path/to/cwd/ /path/to/node_modules/pkg/index.js and the node process is running in the /path/to/cwd/ folder?
If we are cutting off those node_modules lookups that seems odd to me.
Edit: Misunderstood this was a relative code path, all seems good.
guybedford left a comment
guybedford left a comment There was a problem hiding this comment.
Seems good. Missed that this code path is specifically for relative modules (seems I incorrectly expected that resolveLookupPaths would never even apply to relative paths in the first place).
nodejs-github-bot commented Dec 9, 2019
Fixes: #30808 PR-URL: #30835 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Rich Trott <[email protected]>
ZYSzys commented Dec 10, 2019
Landed in 7629fb2. |
Fixes: #30808 PR-URL: #30835 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Fixes: #30808 PR-URL: #30835 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Fixes: #30808 PR-URL: #30835 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Rich Trott <[email protected]>
5 participants
Fixes: #30808
In REPL,
module.filenameisnull, and for relative path modules, we shouldn't look up relative modules fromnode_modules.Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes