Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix dynamic Import issue with function-defined externals #7060

Merged
merged 2 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,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)