Skip to content

Commit

Permalink
fix(cli/repl): use a default referrer when empty (denoland#7794)
Browse files Browse the repository at this point in the history
This makes use of a default referrer when its empty in repl mode so that
dynamic imports work in the global evaluation context.

Co-authored-by: Bartek Iwanczuk <biwanczuk@gmail.com>
  • Loading branch information
2 people authored and JavascriptMick committed Oct 5, 2020
1 parent 454de99 commit 93c310b
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 12 deletions.
14 changes: 14 additions & 0 deletions cli/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,23 @@ impl CliModuleLoader {
impl ModuleLoader for CliModuleLoader {
fn resolve(
&self,
op_state: Rc<RefCell<OpState>>,
specifier: &str,
referrer: &str,
is_main: bool,
) -> Result<ModuleSpecifier, AnyError> {
let global_state = {
let state = op_state.borrow();
state.borrow::<Arc<GlobalState>>().clone()
};

// FIXME(bartlomieju): hacky way to provide compatibility with repl
let referrer = if referrer.is_empty() && global_state.flags.repl {
"<unknown>"
} else {
referrer
};

if !is_main {
if let Some(import_map) = &self.import_map {
let result = import_map.resolve(specifier, referrer)?;
Expand All @@ -58,6 +71,7 @@ impl ModuleLoader for CliModuleLoader {
}
}
}

let module_specifier =
ModuleSpecifier::resolve_import(specifier, referrer)?;

Expand Down
12 changes: 12 additions & 0 deletions cli/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1252,6 +1252,18 @@ fn repl_test_multiline() {
assert!(err.is_empty());
}

#[test]
fn repl_test_import() {
let (out, _) = util::run_and_collect_output(
true,
"repl",
Some(vec!["import('./subdir/auto_print_hello.ts')"]),
Some(vec![("NO_COLOR".to_owned(), "1".to_owned())]),
false,
);
assert!(out.contains("hello!\n"));
}

#[test]
fn repl_test_eval_unterminated() {
let (out, err) = util::run_and_collect_output(
Expand Down
32 changes: 23 additions & 9 deletions core/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ pub trait ModuleLoader {
/// apply import map for child imports.
fn resolve(
&self,
op_state: Rc<RefCell<OpState>>,
specifier: &str,
referrer: &str,
_is_main: bool,
Expand Down Expand Up @@ -109,6 +110,7 @@ pub(crate) struct NoopModuleLoader;
impl ModuleLoader for NoopModuleLoader {
fn resolve(
&self,
_op_state: Rc<RefCell<OpState>>,
_specifier: &str,
_referrer: &str,
_is_main: bool,
Expand Down Expand Up @@ -207,14 +209,23 @@ impl RecursiveModuleLoad {
pub async fn prepare(self) -> (ModuleLoadId, Result<Self, AnyError>) {
let (module_specifier, maybe_referrer) = match self.state {
LoadState::ResolveMain(ref specifier, _) => {
let spec = match self.loader.resolve(specifier, ".", true) {
Ok(spec) => spec,
Err(e) => return (self.id, Err(e)),
};
let spec =
match self
.loader
.resolve(self.op_state.clone(), specifier, ".", true)
{
Ok(spec) => spec,
Err(e) => return (self.id, Err(e)),
};
(spec, None)
}
LoadState::ResolveImport(ref specifier, ref referrer) => {
let spec = match self.loader.resolve(specifier, referrer, false) {
let spec = match self.loader.resolve(
self.op_state.clone(),
specifier,
referrer,
false,
) {
Ok(spec) => spec,
Err(e) => return (self.id, Err(e)),
};
Expand Down Expand Up @@ -243,11 +254,13 @@ impl RecursiveModuleLoad {
fn add_root(&mut self) -> Result<(), AnyError> {
let module_specifier = match self.state {
LoadState::ResolveMain(ref specifier, _) => {
self.loader.resolve(specifier, ".", true)?
}
LoadState::ResolveImport(ref specifier, ref referrer) => {
self.loader.resolve(specifier, referrer, false)?
self
.loader
.resolve(self.op_state.clone(), specifier, ".", true)?
}
LoadState::ResolveImport(ref specifier, ref referrer) => self
.loader
.resolve(self.op_state.clone(), specifier, referrer, false)?,

_ => unreachable!(),
};
Expand Down Expand Up @@ -571,6 +584,7 @@ mod tests {
impl ModuleLoader for MockLoader {
fn resolve(
&self,
_op_state: Rc<RefCell<OpState>>,
specifier: &str,
referrer: &str,
_is_root: bool,
Expand Down
14 changes: 11 additions & 3 deletions core/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ impl JsRuntimeState {
let referrer = self.modules.get_name(referrer_id).unwrap();
let specifier = self
.loader
.resolve(specifier, referrer, false)
.resolve(self.op_state.clone(), specifier, referrer, false)
.expect("Module should have been already resolved");
self.modules.get_id(specifier.as_str()).unwrap_or(0)
}
Expand Down Expand Up @@ -813,8 +813,12 @@ impl JsRuntime {
let import_specifier =
module.get_module_request(i).to_rust_string_lossy(tc_scope);
let state = state_rc.borrow();
let module_specifier =
state.loader.resolve(&import_specifier, name, false)?;
let module_specifier = state.loader.resolve(
state.op_state.clone(),
&import_specifier,
name,
false,
)?;
import_specifiers.push(module_specifier);
}

Expand Down Expand Up @@ -1917,6 +1921,7 @@ pub mod tests {
impl ModuleLoader for ModsLoader {
fn resolve(
&self,
_op_state: Rc<RefCell<OpState>>,
specifier: &str,
referrer: &str,
_is_main: bool,
Expand Down Expand Up @@ -2029,6 +2034,7 @@ pub mod tests {
impl ModuleLoader for DynImportErrLoader {
fn resolve(
&self,
_op_state: Rc<RefCell<OpState>>,
specifier: &str,
referrer: &str,
_is_main: bool,
Expand Down Expand Up @@ -2091,6 +2097,7 @@ pub mod tests {
impl ModuleLoader for DynImportOkLoader {
fn resolve(
&self,
_op_state: Rc<RefCell<OpState>>,
specifier: &str,
referrer: &str,
_is_main: bool,
Expand Down Expand Up @@ -2219,6 +2226,7 @@ pub mod tests {
impl ModuleLoader for ModsLoader {
fn resolve(
&self,
_op_state: Rc<RefCell<OpState>>,
specifier: &str,
referrer: &str,
_is_main: bool,
Expand Down

0 comments on commit 93c310b

Please sign in to comment.