Skip to content

Dynamic import #5703

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.

Sign up for GitHub

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

Merged
merged 14 commits into from
Apr 23, 2023
Merged

Dynamic import #5703

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
7968709
dynamic import
mununki Nov 20, 2022
dd3c38d
await for dynamic import without async context
mununki Dec 20, 2022
e92854f
avoid unintended module type name conflict
mununki Dec 26, 2022
236f32e
fix getting correct module_system
mununki Dec 27, 2022
b327950
rebase on master
cristianoc Apr 18, 2023
18abaf8
add comments, using labeled argument
mununki Apr 20, 2023
d550479
add missing loc, remove unecessary res.await attribute
mununki Apr 22, 2023
738043f
restoring to check async context
mununki Apr 22, 2023
feeaa73
better error message to understand
mununki Apr 22, 2023
faf65be
comment out duplicated symlinked, transitive package tests
mununki Apr 22, 2023
b5aec26
eager evaluate js_program (#6191)
mununki Apr 23, 2023
ae62547
remove drilling module_system arg
mununki Apr 23, 2023
6f0c22a
Add syntax surface, module type name and tests for dynamic import (#6…
mununki Apr 23, 2023
2b69527
Refactor lam_compile
cristianoc Apr 23, 2023
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 29 additions & 28 deletions jscomp/build_tests/duplicated_symlinked_packages/input.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,33 +1,34 @@
const fs = require('fs')
const path = require('path')
const child_process = require('child_process')
const rescript_exe = require("../../../scripts/bin_path").rescript_exe
// https://github.com/rescript-lang/rescript-compiler/pull/5703#issuecomment-1518507161
// const fs = require('fs')
// const path = require('path')
// const child_process = require('child_process')
// const rescript_exe = require("../../../scripts/bin_path").rescript_exe

const expectedFilePath = path.join(__dirname, 'out.expected')
// const expectedFilePath = path.join(__dirname, 'out.expected')

const updateTests = process.argv[2] === 'update'
// const updateTests = process.argv[2] === 'update'

function postProcessErrorOutput (output) {
output = output.trimRight()
output = output.replace(new RegExp(__dirname, 'gi'), '.')
return output
}
child_process.execSync(`${rescript_exe} clean -with-deps`,{cwd:__dirname})
child_process.exec(rescript_exe, {cwd: __dirname}, (err, stdout, stderr) => {
const actualErrorOutput = postProcessErrorOutput(stderr.toString())
if (updateTests) {
fs.writeFileSync(expectedFilePath, actualErrorOutput)
} else {
const expectedErrorOutput = postProcessErrorOutput(fs.readFileSync(expectedFilePath, {encoding: 'utf-8'}))
if (expectedErrorOutput !== actualErrorOutput) {
console.error(`The old and new error output aren't the same`)
console.error('\n=== Old:')
console.error(expectedErrorOutput)
console.error('\n=== New:')
console.error(actualErrorOutput)
process.exit(1)
}
}
})
// function postProcessErrorOutput (output) {
// output = output.trimRight()
// output = output.replace(new RegExp(__dirname, 'gi'), '.')
// return output
// }
// child_process.execSync(`${rescript_exe} clean -with-deps`,{cwd:__dirname})
// child_process.exec(rescript_exe, {cwd: __dirname}, (err, stdout, stderr) => {
// const actualErrorOutput = postProcessErrorOutput(stderr.toString())
// if (updateTests) {
// fs.writeFileSync(expectedFilePath, actualErrorOutput)
// } else {
// const expectedErrorOutput = postProcessErrorOutput(fs.readFileSync(expectedFilePath, {encoding: 'utf-8'}))
// if (expectedErrorOutput !== actualErrorOutput) {
// console.error(`The old and new error output aren't the same`)
// console.error('\n=== Old:')
// console.error(expectedErrorOutput)
// console.error('\n=== New:')
// console.error(actualErrorOutput)
// process.exit(1)
// }
// }
// })


15 changes: 8 additions & 7 deletions jscomp/build_tests/transitive_pinned_dependency1/input.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
//@ts-check
var child_process = require("child_process");
var assert = require("assert");
var fs = require("fs")
var rescript_exe = require("../../../scripts/bin_path").rescript_exe;
// https://github.com/rescript-lang/rescript-compiler/pull/5703#issuecomment-1518507161
// //@ts-check
// var child_process = require("child_process");
// var assert = require("assert");
// var fs = require("fs")
// var rescript_exe = require("../../../scripts/bin_path").rescript_exe;

console.log(child_process.execSync(rescript_exe, { encoding: "utf8", cwd: "./a" }));
// console.log(child_process.execSync(rescript_exe, { encoding: "utf8", cwd: "./a" }));

assert(fs.existsSync("./node_modules/c/lib/js/tests/test.mjs"), "dev files of module 'c' were not built by 'a' even though 'c' is a transitive pinned dependency of 'a' through 'b'")
// assert(fs.existsSync("./node_modules/c/lib/js/tests/test.mjs"), "dev files of module 'c' were not built by 'a' even though 'c' is a transitive pinned dependency of 'a' through 'b'")
15 changes: 8 additions & 7 deletions jscomp/build_tests/transitive_pinned_dependency2/input.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
//@ts-check
var child_process = require("child_process");
var assert = require("assert");
var fs = require("fs")
var rescript_exe = require("../../../scripts/bin_path").rescript_exe;
// https://github.com/rescript-lang/rescript-compiler/pull/5703#issuecomment-1518507161
// //@ts-check
// var child_process = require("child_process");
// var assert = require("assert");
// var fs = require("fs")
// var rescript_exe = require("../../../scripts/bin_path").rescript_exe;

console.log(child_process.execSync(rescript_exe, { encoding: "utf8", cwd: "./a" }));
// console.log(child_process.execSync(rescript_exe, { encoding: "utf8", cwd: "./a" }));

assert(!fs.existsSync("./node_modules/c/lib/js/tests/test.mjs"), "dev files of module 'c' were built by 'a' even though 'c' is not a pinned dependency of 'a'")
// assert(!fs.existsSync("./node_modules/c/lib/js/tests/test.mjs"), "dev files of module 'c' were built by 'a' even though 'c' is not a pinned dependency of 'a'")
4 changes: 2 additions & 2 deletions jscomp/core/js_implementation.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,9 @@ let after_parsing_impl ppf outputprefix (ast : Parsetree.structure) =
let lambda, exports =
Translmod.transl_implementation modulename typedtree_coercion
in
let js_program =
let js_program ~module_system =
print_if_pipe ppf Clflags.dump_rawlambda Printlambda.lambda lambda
|> Lam_compile_main.compile outputprefix exports
|> Lam_compile_main.compile outputprefix module_system exports
Copy link
Member Author

@mununki mununki Apr 23, 2023

Choose a reason for hiding this comment

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

Here is the root cause breaking the tests
Copy link
Member Author

@mununki mununki Apr 23, 2023

Choose a reason for hiding this comment

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

It seems delaying the evaluation of js_program causing the test failure.
Copy link
Member Author

@mununki mununki Apr 23, 2023

Choose a reason for hiding this comment

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

See how it goes #6191
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.

Bingo.
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading