Skip to content

repl: fix current path for dynamic module imports #30060

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 2 commits into from

Conversation

eemeli
Copy link

@eemeli eemeli commented Oct 22, 2019

There was a terminal / missing from the current path when calling import() from the REPL. This means that in a dir /foo/bar calling import('./dog.js') will look for /foo/dog.js, even though it should be looking for /foo/bar/dog.js.

Here's a minimal reproduction:

docker run -it --rm node:12.13.0 bash

mkdir -p /tmp/foo/bar
cd /tmp/foo/bar
echo "console.log('Yes, this is dog')" > dog.js
echo "import('./dog.js')" > call.js

node --experimental-modules call.js
(node:8) ExperimentalWarning: The ESM module loader is experimental.
Yes, this is dog

node --experimental-modules
Welcome to Node.js v12.13.0.
Type ".help" for more information.
> (node:15) ExperimentalWarning: The ESM module loader is experimental.

> import('./dog.js').then(console.log, console.error)
Promise { <pending> }
> Error: Cannot find module /tmp/foo/dog.js imported from /tmp/foo/bar
    at Loader.resolve [as _resolve] (internal/modules/esm/default_resolve.js:82:13)
    at Loader.resolve (internal/modules/esm/loader.js:73:33)
    at Loader.getModuleJob (internal/modules/esm/loader.js:147:40)
    at Loader.import (internal/modules/esm/loader.js:131:28)
    at vm.createScript.importModuleDynamically (repl.js:343:59)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async importModuleDynamicallyWrapper (internal/vm/source_text_module.js:292:17) {
  code: 'ERR_MODULE_NOT_FOUND'
}

> import(path.resolve('./dog.js')).then(console.log, console.error)
Promise { <pending> }
> Yes, this is dog
[Module] { default: {} }
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

Adds a trailing slash to the current directory's path that's used as the
second argument of `asyncESM.ESMLoader.import(specifier, pwd)`. Without
the slash, relative paths will incorrectly resolve from one directory
level higher up.
@nodejs-github-bot nodejs-github-bot added the repl Issues and PRs related to the REPL subsystem. label Oct 22, 2019
@BridgeAR
Copy link
Member

BridgeAR commented Jan 2, 2020

@eemeli thank you very much for your contribution! Seems like this was already fixed by #30609 and the modules team missed to have a look at this. I am very sorry about this, since this looks like great work!

I hope to see further pull requests, since this one can not land anymore.

@BridgeAR BridgeAR closed this Jan 2, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants