diff --git a/CHANGELOG.md b/CHANGELOG.md index 81f76620f80..99c18803c37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ #### :bug: Bug fix - Make rewatch compile independent modules after an unrelated failure and recompile blocked dependents when a changed interface survives a failed implementation, including across full watcher rebuilds. https://github.com/rescript-lang/rescript/pull/8667 +- Fix invalid JavaScript that exported names left unbound when a module's toplevel always throws. https://github.com/rescript-lang/rescript/pull/8692 - Make module inclusion error messages independent of the length of the source file path. https://github.com/rescript-lang/rescript/pull/8691 #### :memo: Documentation diff --git a/compiler/core/lam_compile_main.ml b/compiler/core/lam_compile_main.ml index c30b426c72b..de85a74cc84 100644 --- a/compiler/core/lam_compile_main.ml +++ b/compiler/core/lam_compile_main.ml @@ -63,6 +63,24 @@ let compile_group output_prefix (meta : Lam_stats.t) (x : Lam_group.t) : } lam +(* When a toplevel group always throws, [Js_output.concat] drops every later + group, and a [let] whose right-hand side throws compiles to the [throw] + alone. The identifiers those groups bind are then undeclared, and an export + list naming them is a JavaScript syntax error. This appends a declaration + for each such exported identifier after the [throw]. *) +let declare_undeclared_exports (exports : Ident.t list) (block : J.block) : + J.block = + let declared = + Ext_list.fold_left block Set_ident.empty (fun acc (stmt : J.statement) -> + match stmt.statement_desc with + | Variable {ident} -> Set_ident.add acc ident + | _ -> acc) + in + block + @ Ext_list.filter_map exports (fun id -> + if Set_ident.mem declared id then None + else Some (Js_stmt_make.declare_variable ~kind:Strict id)) + (** Also need analyze its depenency is pure or not *) let no_side_effects (rest : Lam_group.t list) : string option = Ext_list.find_opt rest (fun x -> @@ -355,8 +373,14 @@ let compile (output_prefix : string) export_idents hoisted (lam : Lambda.t) = (Sys.time () *. 1000.) in let body = - Ext_list.map groups (fun group -> compile_group output_prefix meta group) - |> Js_output.concat |> Js_output.output_as_block + let output = + Ext_list.map groups (fun group -> compile_group output_prefix meta group) + |> Js_output.concat + in + let block = Js_output.output_as_block output in + if output.output_finished = True then + declare_undeclared_exports meta.exports block + else block in let () = if debug_ir then diff --git a/tests/tests/src/conditional/cond_a.mjs b/tests/tests/src/conditional/cond_a.mjs index 7eea77289af..e77a609cb87 100644 --- a/tests/tests/src/conditional/cond_a.mjs +++ b/tests/tests/src/conditional/cond_a.mjs @@ -11,6 +11,8 @@ throw { Error: new Error() }; +let u; + export { u, } diff --git a/tests/tests/src/conditional/cond_a_load_test.mjs b/tests/tests/src/conditional/cond_a_load_test.mjs new file mode 100644 index 00000000000..47502b2cc2f --- /dev/null +++ b/tests/tests/src/conditional/cond_a_load_test.mjs @@ -0,0 +1,44 @@ +// Generated by ReScript, PLEASE EDIT WITH CARE + +import * as Mocha from "mocha"; +import * as Pervasives from "@rescript/runtime/lib/es6/Pervasives.mjs"; +import * as Primitive_exceptions from "@rescript/runtime/lib/es6/Primitive_exceptions.mjs"; + +async function loadCondANone() { + let M = await import("./cond_a_none.mjs"); + return M.A.u; +} + +async function raisesAssertFailure(load) { + try { + await load(); + return false; + } catch (raw_exn) { + let exn = Primitive_exceptions.internalToException(raw_exn); + if (exn.RE_EXN_ID === "Assert_failure") { + return true; + } + throw exn; + } +} + +Mocha.describe("Cond_a_load_test", () => { + Mocha.test("cond_a loads as a module whose body throws Assert_failure", async () => { + let ok = await raisesAssertFailure(() => import("./cond_a.mjs").then(m => m.u)); + if (!ok) { + return Pervasives.failwith("cond_a did not throw Assert_failure on load"); + } + }); + Mocha.test("cond_a_none loads as a module whose body throws Assert_failure", async () => { + let ok = await raisesAssertFailure(loadCondANone); + if (!ok) { + return Pervasives.failwith("cond_a_none did not throw Assert_failure on load"); + } + }); +}); + +export { + loadCondANone, + raisesAssertFailure, +} +/* Not a pure module */ diff --git a/tests/tests/src/conditional/cond_a_load_test.res b/tests/tests/src/conditional/cond_a_load_test.res new file mode 100644 index 00000000000..407d13ca0b3 --- /dev/null +++ b/tests/tests/src/conditional/cond_a_load_test.res @@ -0,0 +1,30 @@ +open Mocha + +@module("mocha") +external testAsync: (string, unit => promise) => unit = "test" + +let loadCondANone = async () => { + module M = await Cond_a_none + M.A.u +} + +let raisesAssertFailure = async load => + switch await load() { + | _ => false + | exception Assert_failure(_) => true + } + +describe(__MODULE__, () => { + testAsync("cond_a loads as a module whose body throws Assert_failure", async () => { + let ok = await raisesAssertFailure(() => import(Cond_a.u)) + if !ok { + failwith("cond_a did not throw Assert_failure on load") + } + }) + testAsync("cond_a_none loads as a module whose body throws Assert_failure", async () => { + let ok = await raisesAssertFailure(loadCondANone) + if !ok { + failwith("cond_a_none did not throw Assert_failure on load") + } + }) +}) diff --git a/tests/tests/src/conditional/cond_a_none.mjs b/tests/tests/src/conditional/cond_a_none.mjs index 765ecc86fd8..21d625e8ad4 100644 --- a/tests/tests/src/conditional/cond_a_none.mjs +++ b/tests/tests/src/conditional/cond_a_none.mjs @@ -11,6 +11,8 @@ throw { Error: new Error() }; +let A; + export { A, }