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

Dynamic import #5703

Merged
merged 14 commits into from
Apr 23, 2023
Merged
2 changes: 1 addition & 1 deletion jscomp/core/js_implementation.ml
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ let after_parsing_impl ppf outputprefix (ast : Parsetree.structure) =
in
let js_program =
print_if_pipe ppf Clflags.dump_rawlambda Printlambda.lambda lambda
|> Lam_compile_main.compile outputprefix exports
|> Lam_compile_main.compile outputprefix NodeJS exports
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about passing NodeJS as always here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That does not sound right. Unless, this is only used by the code you have added?
Can you explain the context more here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can try using Js_packages_info.t inside lam_compile_primitive directly where the module_system needs to know.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That does not sound right. Unless, this is only used by the code you have added? Can you explain the context more here?

Pimport needs to know about the information what module_system is. That's why the argument drilling was all over the place in lam along with output_prefix. We can leave the output_prefix but module_system.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would help to first reduce the number of lines changes. As right now it's really hard to see where this is used.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eg. instead of
let rec foo x y ... and bar x y ... and ...
do

let toplevel x y =
let rec foo ... and bar ...

So most of the change becomes only indentation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeap, let me try experiments to get module_system from Js_packages_info inside Pimport.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeap, let me try experiments to get module_system from Js_packages_info inside Pimport.

Yes that would clean all of this, if it works.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side comment: The previous design did not require a module_system when writing a js_program with lam because it only needed a module_system to represent the import(top of file) and export(bottom of file) of the js output. However, the dynamic import feature requires a module_system in the js_program as well.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side side comment. I wonder for how long we'll need to support all these module systems. It would be nice to converge to es6 at some point.

in
if not !Js_config.cmj_only then
Lam_compile_main.lambda_as_module js_program outputprefix);
Expand Down
2 changes: 2 additions & 0 deletions jscomp/core/js_packages_info.ml
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ let is_runtime_package (x : t) = x.name = Pkg_runtime

let iter (x : t) cb = Ext_list.iter x.module_systems cb

let map (x : t) cb = Ext_list.map x.module_systems cb

(* let equal (x : t) ({name; module_systems}) =
x.name = name &&
Ext_list.for_all2_no_exn
Expand Down
2 changes: 2 additions & 0 deletions jscomp/core/js_packages_info.mli
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ val same_package_by_name : t -> t -> bool

val iter : t -> (package_info -> unit) -> unit

val map : t -> (package_info -> 'a) -> 'a list

val empty : t

val from_name : string -> t
Expand Down
2 changes: 1 addition & 1 deletion jscomp/core/lam_analysis.ml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ let rec no_side_effects (lam : Lam.t) : bool =
| Pcreate_extension _ | Pjs_typeof | Pis_null | Pis_not_none | Psome
| Psome_not_nest | Pis_undefined | Pis_null_undefined | Pnull_to_opt
| Pundefined_to_opt | Pnull_undefined_to_opt | Pjs_fn_make _ | Pjs_fn_make_unit
| Pjs_object_create _
| Pjs_object_create _ | Pimport
(* TODO: check *)
| Pbytes_to_string | Pmakeblock _
(* whether it's mutable or not *)
Expand Down
273 changes: 141 additions & 132 deletions jscomp/core/lam_compile.ml

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions jscomp/core/lam_compile.mli
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,6 @@
(** Compile single lambda IR to JS IR *)

val compile_recursive_lets :
Lam_compile_context.t -> (Ident.t * Lam.t) list -> Js_output.t
output_prefix:string -> Js_packages_info.module_system -> Lam_compile_context.t -> (Ident.t * Lam.t) list -> Js_output.t
Copy link
Collaborator

@cristianoc cristianoc Apr 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About invasive:

This and the function below are the only 2 entry points where the 2 new parameters are passed in.
No need to thread them though each recursive call if they never change and are passed from the outside.

Copy link
Collaborator

@cristianoc cristianoc Apr 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once the number of changes is reduced, it should be possible to just isolate what change is responsible.
E.g. perhaps first remove Pimport, then remove gradually every other change in turn, locally, until the tests stops failing and one finds out the root cause.


val compile_lambda : Lam_compile_context.t -> Lam.t -> Js_output.t
val compile_lambda : output_prefix:string -> Js_packages_info.module_system -> Lam_compile_context.t -> Lam.t -> Js_output.t
17 changes: 9 additions & 8 deletions jscomp/core/lam_compile_main.ml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
(* module S = Js_stmt_make *)


let compile_group (meta : Lam_stats.t)
let compile_group output_prefix module_system (meta : Lam_stats.t)
(x : Lam_group.t) : Js_output.t =
match x with
(*
Expand All @@ -60,20 +60,20 @@ let compile_group (meta : Lam_stats.t)
(* let lam = Optimizer.simplify_lets [] lam in *)
(* can not apply again, it's wrong USE it with care*)
(* ([Js_stmt_make.comment (Gen_of_env.query_type id env )], None) ++ *)
Lam_compile.compile_lambda { continuation = Declare (kind, id);
Lam_compile.compile_lambda ~output_prefix module_system { continuation = Declare (kind, id);
jmp_table = Lam_compile_context.empty_handler_map;
meta
} lam

| Recursive id_lams ->
Lam_compile.compile_recursive_lets
Lam_compile.compile_recursive_lets ~output_prefix module_system
{ continuation = EffectCall Not_tail;
jmp_table = Lam_compile_context.empty_handler_map;
meta
}
id_lams
| Nop lam -> (* TODO: Side effect callls, log and see statistics *)
Lam_compile.compile_lambda {continuation = EffectCall Not_tail;
Lam_compile.compile_lambda ~output_prefix module_system {continuation = EffectCall Not_tail;
jmp_table = Lam_compile_context.empty_handler_map;
meta
} lam
Expand Down Expand Up @@ -122,7 +122,8 @@ let _j = Js_pass_debug.dump
it's used or not
*)
let compile
(output_prefix : string)
(output_prefix : string)
(module_system : Js_packages_info.module_system)
export_idents
(lam : Lambda.lambda) =
let export_ident_sets = Set_ident.of_list export_idents in
Expand Down Expand Up @@ -222,7 +223,7 @@ let maybe_pure = no_side_effects groups in
let () = Ext_log.dwarn ~__POS__ "\n@[[TIME:]Pre-compile: %f@]@." (Sys.time () *. 1000.) in
#endif
let body =
Ext_list.map groups (fun group -> compile_group meta group)
Ext_list.map groups (fun group -> compile_group output_prefix module_system meta group)
|> Js_output.concat
|> Js_output.output_as_block
in
Expand Down Expand Up @@ -292,13 +293,13 @@ let lambda_as_module
: unit =
let package_info = Js_packages_state.get_packages_info () in
if Js_packages_info.is_empty package_info && !Js_config.js_stdout then begin
Js_dump_program.dump_deps_program ~output_prefix NodeJS lambda_output stdout
Js_dump_program.dump_deps_program ~output_prefix NodeJS (lambda_output) stdout
end else
Js_packages_info.iter package_info (fun {module_system; path; suffix} ->
let output_chan chan =
Js_dump_program.dump_deps_program ~output_prefix
module_system
lambda_output
(lambda_output)
chan in
let basename =
Ext_namespace.change_ext_ns_suffix
Expand Down
2 changes: 1 addition & 1 deletion jscomp/core/lam_compile_main.mli
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
(** Compile and register the hook of function to compile a lambda to JS IR
*)

val compile : string -> Ident.t list -> Lambda.lambda -> J.deps_program
val compile : string -> Js_packages_info.module_system -> Ident.t list -> Lambda.lambda -> J.deps_program
(** For toplevel, [filename] is [""] which is the same as
{!Env.get_unit_name ()}
*)
Expand Down
54 changes: 52 additions & 2 deletions jscomp/core/lam_compile_primitive.ml
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,33 @@ let ensure_value_unit (st : Lam_compile_context.continuation) e : E.t =
| EffectCall Not_tail -> e
(* NeedValue should return a meaningful expression*)

let translate loc (cxt : Lam_compile_context.t) (prim : Lam_primitive.t)
(args : J.expression list) : J.expression =
let module_of_expression = function
| J.Var (J.Qualified (module_id, value)) -> [ (module_id, value) ]
| _ -> []

let import_of_path path =
E.call
~info:{ arity = Full; call_info = Call_na }
(E.js_global "import")
[ E.str path ]

let wrap_then import value =
let arg = Ident.create "m" in
E.call
~info:{ arity = Full; call_info = Call_na }
(E.dot import "then")
[
E.ocaml_fun ~return_unit:false ~async:false ~oneUnitArg:false [ arg ]
[
{
statement_desc = J.Return (E.dot (E.var arg) value);
comment = None;
};
];
]

let translate output_prefix module_system loc (cxt : Lam_compile_context.t)
(prim : Lam_primitive.t) (args : J.expression list) : J.expression =
match prim with
| Pis_not_none -> Js_of_lam_option.is_not_none (Ext_list.singleton_exn args)
| Pcreate_extension s -> E.make_exception s
Expand Down Expand Up @@ -78,6 +103,31 @@ let translate loc (cxt : Lam_compile_context.t) (prim : Lam_primitive.t)
| _ -> E.runtime_call Js_runtime_modules.option "nullable_to_opt" args
)
| _ -> assert false)
(* Compile #import: The module argument for dynamic import is represented as a path,
and the module value is expressed through wrapping it with promise.then *)
| Pimport -> (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add comment describing what the transformation does.

match args with
| [ e ] -> (
let output_dir = Filename.dirname output_prefix in

let module_id, module_value =
match module_of_expression e.expression_desc with
| [ module_ ] -> module_
| _ -> Location.raise_errorf ~loc
"Invalid argument: Dynamic import requires a module or module value that is a file as argument. Passing a value or local module is not allowed."
in

let path =
Js_name_of_module_id.string_of_module_id module_id ~output_dir
module_system
in

match module_value with
| Some value -> wrap_then (import_of_path path) value
| None -> import_of_path path)
| [] | _ ->
Location.raise_errorf ~loc
"Invalid argument: Dynamic import must take a single module or module value as its argument.")
| Pjs_function_length -> E.function_length (Ext_list.singleton_exn args)
| Pcaml_obj_length -> E.obj_length (Ext_list.singleton_exn args)
| Pis_null -> E.is_null (Ext_list.singleton_exn args)
Expand Down
2 changes: 2 additions & 0 deletions jscomp/core/lam_compile_primitive.mli
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
*)

val translate :
string ->
Js_packages_info.module_system ->
Location.t ->
Lam_compile_context.t ->
Lam_primitive.t ->
Expand Down
1 change: 1 addition & 0 deletions jscomp/core/lam_convert.ml
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,7 @@ let convert (exports : Set_ident.t) (lam : Lambda.lambda) :
| "#nullable_to_opt" -> Pnull_undefined_to_opt
| "#null_to_opt" -> Pnull_to_opt
| "#is_nullable" -> Pis_null_undefined
| "#import" ->Pimport
| "#string_append" -> Pstringadd
| "#wrap_exn" -> Pwrap_exn
| "#obj_length" -> Pcaml_obj_length
Expand Down
2 changes: 2 additions & 0 deletions jscomp/core/lam_primitive.ml
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ type t =
| Pis_null
| Pis_undefined
| Pis_null_undefined
| Pimport
| Pjs_typeof
| Pjs_function_length
| Pcaml_obj_length
Expand Down Expand Up @@ -219,6 +220,7 @@ let eq_primitive_approx (lhs : t) (rhs : t) =
| Psome_not_nest -> rhs = Psome_not_nest
| Pis_undefined -> rhs = Pis_undefined
| Pis_null_undefined -> rhs = Pis_null_undefined
| Pimport -> rhs = Pimport
| Pjs_typeof -> rhs = Pjs_typeof
| Pisint -> rhs = Pisint
| Pis_poly_var_block -> rhs = Pis_poly_var_block
Expand Down
1 change: 1 addition & 0 deletions jscomp/core/lam_primitive.mli
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ type t =
| Pis_null
| Pis_undefined
| Pis_null_undefined
| Pimport
| Pjs_typeof
| Pjs_function_length
| Pcaml_obj_length
Expand Down
1 change: 1 addition & 0 deletions jscomp/core/lam_print.ml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ let primitive ppf (prim : Lam_primitive.t) =
| Pval_from_option_not_nest -> fprintf ppf "[?unbox-not-nest]"
| Pis_undefined -> fprintf ppf "[?undefined]"
| Pis_null_undefined -> fprintf ppf "[?null?undefined]"
| Pimport -> fprintf ppf "[import]"
| Pmakeblock (tag, _, Immutable) -> fprintf ppf "makeblock %i" tag
| Pmakeblock (tag, _, Mutable) -> fprintf ppf "makemutable %i" tag
| Pfield (n, field_info) -> (
Expand Down
33 changes: 33 additions & 0 deletions jscomp/frontend/ast_await.ml
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,36 @@ let create_await_expression (e : Parsetree.expression) =
{txt = Ldot (Ldot (Lident "Js", "Promise"), "unsafe_await"); loc}
in
Ast_helper.Exp.apply ~loc unsafe_await [(Nolabel, e)]

(* Transform `@res.await M` to unpack(@res.await Js.import(module(M: __M0__))) *)
let create_await_module_expression ~module_type_name (e : Parsetree.module_expr)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment explaining what this transformation does.

=
let open Ast_helper in
let remove_await_attribute =
List.filter (fun ((loc, _) : Parsetree.attribute) -> loc.txt != "res.await")
in
{
e with
pmod_desc =
Pmod_unpack
(create_await_expression
(Exp.apply ~loc:e.pmod_loc
(Exp.ident ~loc:e.pmod_loc
{
txt = Longident.Ldot (Lident "Js", "import");
loc = e.pmod_loc;
})
[
( Nolabel,
Exp.constraint_ ~loc:e.pmod_loc
(Exp.pack ~loc:e.pmod_loc
{
e with
pmod_attributes =
remove_await_attribute e.pmod_attributes;
})
(Typ.package ~loc:e.pmod_loc
{txt = Lident module_type_name; loc = e.pmod_loc}
[]) );
]));
}
39 changes: 39 additions & 0 deletions jscomp/frontend/bs_builtin_ppx.ml
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,16 @@ let local_module_name =
incr v;
"local_" ^ string_of_int !v

(* Unpack requires core_type package for type inference;
use module type bindings and a function to create safe local names instead. *)
let local_module_type_name =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add comment about what this does.

let v = ref 0 in
fun ({txt} : Longident.t Location.loc) ->
incr v;
"__"
^ (Longident.flatten txt |> List.fold_left (fun ll l -> ll ^ l) "")
^ string_of_int !v ^ "__"

let expand_reverse (stru : Ast_structure.t) (acc : Ast_structure.t) :
Ast_structure.t =
if stru = [] then acc
Expand Down Expand Up @@ -486,6 +496,35 @@ let rec structure_mapper (self : mapper) (stru : Ast_structure.t) =
| _ -> expand_reverse acc (structure_mapper self rest)
in
aux [] stru
(* Dynamic import of module transformation: module M = @res.await Belt.List *)
| Pstr_module
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment explaining what this transformation does.

({pmb_expr = {pmod_desc = Pmod_ident {txt; loc}; pmod_attributes} as me}
as mb)
when Res_parsetree_viewer.hasAwaitAttribute pmod_attributes ->
let item = self.structure_item self item in
let safe_module_type_name = local_module_type_name {txt; loc} in
let module_type_decl =
let open Ast_helper in
Str.modtype ~loc
(Mtd.mk ~loc
{txt = safe_module_type_name; loc}
~typ:(Mty.typeof_ ~loc me))
in
(* module __BeltList1__ = module type of Belt.List *)
module_type_decl
:: {
item with
pstr_desc =
Pstr_module
{
mb with
pmb_expr =
Ast_await.create_await_module_expression
~module_type_name:safe_module_type_name mb.pmb_expr;
};
}
(* module M = @res.await Belt.List *)
:: structure_mapper self rest
| _ -> self.structure_item self item :: structure_mapper self rest)

let mapper : mapper =
Expand Down
1 change: 1 addition & 0 deletions jscomp/others/js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ external toOption : 'a nullable -> 'a option = "#nullable_to_opt"
external undefinedToOption : 'a undefined -> 'a option = "#undefined_to_opt"
external nullToOption : 'a null -> 'a option = "#null_to_opt"
external isNullable : 'a nullable -> bool = "#is_nullable"
external import : 'a -> 'a promise = "#import"

external testAny : 'a -> bool = "#is_nullable"
(** The same as {!test} except that it is more permissive on the types of input *)
Expand Down
2 changes: 2 additions & 0 deletions jscomp/runtime/js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ external nullToOption : 'a null -> 'a option = "#null_to_opt"

external isNullable : 'a nullable -> bool = "#is_nullable"

external import : 'a -> 'a promise = "#import"

(** The same as {!test} except that it is more permissive on the types of input *)
external testAny : 'a -> bool = "#is_nullable"

Expand Down
Loading