fix: treat dynamic imports as side-effectful (#1172)

* fix: treat dynamic imports as side-effectful * fix dyn import tests
melange-re · Sep 17, 2024 · bc9e2f6 · bc9e2f6
1 parent bfe7554
commit bc9e2f6
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 7 deletions.
diff --git a/jscomp/core/lam_analysis.ml b/jscomp/core/lam_analysis.ml
@@ -74,9 +74,9 @@ 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_object_create _ (* TODO: check *) | Pimport | Pbytes_to_string
-      | Pbytes_of_string | Pmakeblock _ (* whether it's mutable or not *)
-      | Pfield _ | Pfield_computed | Pval_from_option
+      | Pjs_object_create _ | Pbytes_to_string | Pbytes_of_string
+      | Pmakeblock _ (* whether it's mutable or not *) | Pfield _
+      | Pfield_computed | Pval_from_option
       | Pval_from_option_not_nest
         (* NOP The compiler already [t option] is the same as t *)
       | Pduprecord _
@@ -113,7 +113,7 @@ let rec no_side_effects (lam : Lam.t) : bool =
           true
       | Pjs_apply | Pjs_runtime_apply | Pjs_call _ | Pinit_mod | Pupdate_mod
       | Pjs_unsafe_downgrade _ | Pdebugger | Pvoid_run | Pfull_apply
-      | Pjs_fn_method
+      | Pjs_fn_method | Pimport
       (* TODO *)
       | Praw_js_code _ | Pbytessetu | Pbytessets | Pbytes_set_16 _
       | Pbytes_set_32 _ | Pbytes_set_64 _

diff --git a/test/blackbox-tests/dynamic-import-external-fn.t b/test/blackbox-tests/dynamic-import-external-fn.t
@@ -30,7 +30,7 @@ translation converts into LFunction stubs
       });
   
   exports._ext = _ext;
-  /* lib Not a pure module */
+  /* _ext Not a pure module */
 
 Example where the `return_unit` function attribute is `true` and the lambda
 includes `Lsequence (_, Lconst Const_js_undefined)`
@@ -55,7 +55,7 @@ includes `Lsequence (_, Lconst Const_js_undefined)`
       });
   
   exports.dynamicallyImportedUseEffect = dynamicallyImportedUseEffect;
-  /* react Not a pure module */
+  /* dynamicallyImportedUseEffect Not a pure module */
 
 
   $ cat > x.re <<EOF

diff --git a/test/blackbox-tests/dynamic-import-side-effect.t b/test/blackbox-tests/dynamic-import-side-effect.t
@@ -0,0 +1,38 @@
+Demonstrate dynamic `import()` semantics
+
+  $ . ./setup.sh
+
+  $ cat > dune-project <<EOF
+  > (lang dune 3.9)
+  > (using melange 0.1)
+  > EOF
+
+  $ cat > dune <<EOF
+  > (melange.emit
+  >  (target out)
+  >  (preprocess (pps melange.ppx))
+  >  (emit_stdlib false))
+  > EOF
+
+  $ cat > x.ml <<EOF
+  > module type int  = module type of Int
+  > let () =
+  >  let _x = Js.import ((module Stdlib.Int) : (module int)) in
+  >  ()
+  > EOF
+
+  $ cat > y.ml <<EOF
+  > let x = 1
+  > EOF
+
+  $ dune build @melange
+
+  $ cat _build/default/out/x.js
+  // Generated by Melange
+  'use strict';
+  
+  
+  import("melange/int.js");
+  
+  /*  Not a pure module */
+
diff --git a/test/blackbox-tests/dynamic-import.t b/test/blackbox-tests/dynamic-import.t
@@ -57,4 +57,4 @@ Demonstrate dynamic `import()` semantics
   exports.a = a;
   exports.b = b;
   exports._ext = _ext;
-  /* _ext Not a pure module */
+  /* a Not a pure module */