diff --git a/core/engine/src/module/source.rs b/core/engine/src/module/source.rs index c904a9be70a..8ca5aee8f87 100644 --- a/core/engine/src/module/source.rs +++ b/core/engine/src/module/source.rs @@ -29,7 +29,7 @@ use crate::{ Context, JsArgs, JsError, JsExpect, JsNativeError, JsObject, JsResult, JsString, JsValue, NativeFunction, SpannedSourceText, builtins::{Promise, promise::PromiseCapability}, - bytecompiler::{BindingAccessOpcode, ByteCompiler, FunctionSpec, ToJsString}, + bytecompiler::{ByteCompiler, FunctionSpec, ToJsString}, environments::{DeclarativeEnvironment, EnvironmentStack}, job::NativeAsyncJob, js_string, @@ -1662,6 +1662,7 @@ impl SourceTextModule { compiler.async_handler = self.code.has_tla.then(|| compiler.push_handler()); let mut imports = Vec::new(); + let mut vars = Vec::new(); let (codeblock, functions) = { // 7. For each ImportEntry Record in of module.[[ImportEntries]], do @@ -1747,15 +1748,9 @@ impl SourceTextModule { if !declared_var_names.contains(&name) { // 1. Perform ! env.CreateMutableBinding(dn, false). // 2. Perform ! env.InitializeBinding(dn, undefined). - let binding = env - .get_binding_reference(&name) - .js_expect("binding must exist")?; - let index = compiler.insert_binding(binding); - compiler.emit_binding_access( - BindingAccessOpcode::DefInitVar, - &index, - &CallFrame::undefined_register(), - ); + // deferred to initialization below + let locator = env.get_binding(&name).js_expect("binding must exist")?; + vars.push(locator); // 3. Append dn to declaredVarNames. declared_var_names.push(name); @@ -1901,6 +1896,21 @@ impl SourceTextModule { } } + // deferred initialization of var bindings (module-scope bindings always escape in + // `BindingEscapeAnalyzer::visit_module_mut`, so these locators address environment slots) + { + let frame = context.vm.frame_mut(); + let global = frame.realm.environment(); + for locator in vars { + frame.environments.put_lexical_value( + locator.scope(), + locator.binding_index(), + JsValue::undefined(), + global, + ); + } + } + // deferred initialization of function exports for (index, locator) in functions { let code = codeblock.constant_function(index as usize); diff --git a/core/engine/tests/module.rs b/core/engine/tests/module.rs index 9c49ce569e7..80564913e4f 100644 --- a/core/engine/tests/module.rs +++ b/core/engine/tests/module.rs @@ -5,8 +5,11 @@ use std::future; use std::rc::Rc; use boa_engine::builtins::promise::PromiseState; -use boa_engine::module::{ModuleLoader, Referrer}; -use boa_engine::{Context, JsResult, JsString, Module, Source, js_string}; +use boa_engine::module::{MapModuleLoader, ModuleLoader, Referrer}; +use boa_engine::{ + Context, JsNativeError, JsNativeErrorKind, JsResult, JsString, JsValue, Module, Source, + js_string, +}; #[test] fn test_json_module_from_str() { @@ -192,7 +195,7 @@ fn test_json_module_static_import_with_attributes() { assert_eq!( promise.state(), - PromiseState::Fulfilled(boa_engine::JsValue::undefined()) + PromiseState::Fulfilled(JsValue::undefined()) ); let value = module @@ -248,7 +251,7 @@ fn test_json_module_reexport_with_attributes() { assert_eq!( promise.state(), - PromiseState::Fulfilled(boa_engine::JsValue::undefined()) + PromiseState::Fulfilled(JsValue::undefined()) ); let json = module @@ -452,3 +455,105 @@ fn test_dynamic_import_symbol_key() { PromiseState::Pending => panic!("Dynamic import is still pending"), } } + +/// Linking a module initializes its `var` bindings to `undefined`, while its lexical bindings stay +/// uninitialized until its body runs. +#[test] +fn test_module_var_bindings_are_initialized_at_link_time() { + let mut context = Context::default(); + let module = Module::parse( + Source::from_bytes("export var x = 1; export let y = 2;"), + None, + &mut context, + ) + .unwrap(); + + let promise = module.load(&mut context); + context.run_jobs().unwrap(); + assert!(promise.state().as_fulfilled().is_some()); + module.link(&mut context).unwrap(); + + let namespace = module.namespace(&mut context); + assert_eq!( + namespace.get(js_string!("x"), &mut context).unwrap(), + JsValue::undefined() + ); + let error = namespace.get(js_string!("y"), &mut context).unwrap_err(); + assert_eq!( + error.as_native().map(JsNativeError::kind), + Some(&JsNativeErrorKind::Reference) + ); +} + +/// Links and evaluates the module `a`, which imports the module `b`, which imports `a` back, and +/// returns `a`'s export named `result`. +/// +/// `b`'s body runs before `a`'s, because `b`'s import of `a` finds `a` already evaluating. +fn evaluate_cycle(a: &str, b: &str) -> JsValue { + let loader = Rc::new(MapModuleLoader::new()); + let mut context = Context::builder() + .module_loader(loader.clone()) + .build() + .unwrap(); + + let a = Module::parse(Source::from_bytes(a), None, &mut context).unwrap(); + let b = Module::parse(Source::from_bytes(b), None, &mut context).unwrap(); + loader.insert("a", a.clone()); + loader.insert("b", b); + + let promise = a.load_link_evaluate(&mut context); + context.run_jobs().unwrap(); + if let PromiseState::Rejected(reason) = promise.state() { + panic!("module evaluation failed: {}", reason.display()); + } + assert_eq!( + promise.state(), + PromiseState::Fulfilled(JsValue::undefined()) + ); + + a.namespace(&mut context) + .get(js_string!("result"), &mut context) + .unwrap() +} + +/// A module's `var` bindings are initialized to `undefined` when the module is linked, so code that +/// runs before the module's body reads them as `undefined`. +#[test] +fn test_module_var_bindings_read_before_evaluation_are_undefined() { + let result = evaluate_cycle( + r#" + import { observed } from "b"; + export var exported = 1; + var local = 2; + export function readLocal() { return local; } + export const result = observed; + "#, + r#" + import { exported, readLocal } from "a"; + import * as a from "a"; + export const observed = [exported, a.exported, readLocal()].map(String).join(); + "#, + ); + + assert_eq!(result, js_string!("undefined,undefined,undefined").into()); +} + +/// A module's body does not reinitialize its `var` bindings, so a value written to one before the +/// body runs is kept. +#[test] +fn test_module_var_binding_written_before_evaluation_keeps_its_value() { + let result = evaluate_cycle( + r#" + import "b"; + export var value; + export function setValue(v) { value = v; } + export const result = value; + "#, + r#" + import { setValue } from "a"; + setValue(5); + "#, + ); + + assert_eq!(result, JsValue::from(5)); +}