-
Notifications
You must be signed in to change notification settings - Fork 455
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
Dynamic import #5703
Changes from 11 commits
7968709
dd3c38d
e92854f
236f32e
b327950
18abaf8
d550479
738043f
feeaa73
faf65be
b5aec26
ae62547
6f0c22a
2b69527
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
|
||
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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -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 -> ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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} | ||
[]) ); | ||
])); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 = | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
insidelam_compile_primitive
directly where the module_system needs to know.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that would clean all of this, if it works.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.