Skip to content

Commit

Permalink
Fix dynamic Import issue with function-defined externals (#7060)
Browse files Browse the repository at this point in the history
* fix dynamic import for Lfunction

* changelog
  • Loading branch information
mununki authored Oct 1, 2024
1 parent ac30044 commit 70700fe
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

- Use FORCE_COLOR environmental variable to force colorized output https://github.com/rescript-lang/rescript-compiler/pull/7033
- Allow spreads of variants in patterns (`| ...someVariant as v => `) when the variant spread is a subtype of the variant matched on. https://github.com/rescript-lang/rescript-compiler/pull/6721
- Fix the issue where dynamic imports are not working for function-defined externals. https://github.com/rescript-lang/rescript-compiler/pull/7060

#### :bug: Bug fix

Expand Down
54 changes: 54 additions & 0 deletions jscomp/core/lam_compile.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1625,6 +1625,60 @@ and compile_prim (prim_info : Lam.prim_info)
Js_output.output_of_block_and_expression lambda_cxt.continuation
(Ext_list.concat_append args_block block)
exp
| { primitive = Pimport; args = [] | _ :: _ :: _; loc } ->
Location.raise_errorf ~loc
"Missing argument: Dynamic import requires a module or \
module value that is a file as argument."
| { primitive = Pimport as primitive; args = [ mod_ ]; loc} ->
(match mod_ with
| Lglobal_module _ | Lvar _
| Lprim { primitive = Pfield _ | Pjs_call _ ; _ } ->
let args_block, args_expr =
let new_cxt =
{ lambda_cxt with continuation = NeedValue Not_tail }
in
match compile_lambda new_cxt mod_ with
| { block; value = Some b; _ } -> ([ block ], b)
| { value = None; _ } -> assert false
in
let args_code : J.block = List.concat args_block in
let exp =
Lam_compile_primitive.translate output_prefix loc lambda_cxt primitive [args_expr]
in
Js_output.output_of_block_and_expression lambda_cxt.continuation args_code
exp
| Lfunction {
body =
( (Lprim _ as body)
| Lsequence ((Lprim _ as body), Lconst Const_js_undefined _) );
_;
} ->
let body = match body with
| Lprim ({ primitive = Pjs_call prim_info; args; loc }) ->
Lam.prim
~primitive:(Lam_primitive.Pjs_call { prim_info with dynamic_import = true })
~args
loc
| _ -> body
in
let args_block, args_expr =
let new_cxt =
{ lambda_cxt with continuation = NeedValue Not_tail }
in
match compile_lambda new_cxt body with
| { block; value = Some b; _ } -> ([ block ], b)
| { value = None; _ } -> assert false
in
let args_code : J.block = List.concat args_block in
let exp =
Lam_compile_primitive.translate output_prefix loc lambda_cxt primitive [args_expr]
in
Js_output.output_of_block_and_expression lambda_cxt.continuation args_code
exp
| _ ->
Location.raise_errorf ~loc
"Invalid argument: unsupported argument to dynamic import. If \
you believe this should be supported, please open an issue.")
| { primitive; args; loc } ->
let args_block, args_expr =
if args = [] then ([], [])
Expand Down
1 change: 1 addition & 0 deletions jscomp/core/lam_compile_primitive.ml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ let ensure_value_unit (st : Lam_compile_context.continuation) e : E.t =

let module_of_expression = function
| J.Var (J.Qualified (module_id, value)) -> [ (module_id, value) ]
| J.Call ({expression_desc = (J.Var (J.Qualified (module_id, value)))}, _, _) -> [ (module_id, value) ]
| _ -> []

let get_module_system () =
Expand Down
3 changes: 3 additions & 0 deletions jscomp/test/import_external.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions jscomp/test/import_external.res
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,8 @@
external makeA: string = "default"

let f8 = Js.import(makeA)

@module("b")
external makeB: string => unit = "default"

let f9 = Js.import(makeB)

0 comments on commit 70700fe

Please sign in to comment.