Skip to content

Add syntax surface, module type name and tests for dynamic import #6188

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

Jump to bottom
Merged
merged 8 commits into from
Apr 23, 2023
Merged

Add syntax surface, module type name and tests for dynamic import #6188

merged 8 commits into from
Apr 23, 2023

Conversation

mununki
Copy link
Member

@mununki mununki commented Apr 22, 2023 •
edited
Loading

  • Safe module type name
  • Adding syntax surface for dynamic import.
// for value
let forEach = await @a @b Js.Import(Belt.List.forEach)

// for module
module M = await @a @b Belt.List
  • Test pass: duplicated symlinked packages, transitive pinned packages

@mununki mununki changed the base branch from master to import-mwk April 22, 2023 06:53
@mununki mununki changed the title Add syntax surface and tests for dynamic import Add syntax surface, module type name and tests for dynamic import Apr 22, 2023
cristianoc
cristianoc reviewed Apr 22, 2023
View reviewed changes
jscomp/frontend/bs_builtin_ppx.ml
(* Unpack requires core_type package for type inference:
Generate a module type name eg. __Belt_List__*)
let local_module_type_name txt =
"_"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why single underscore before and double afterwards ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because I add seperator "_" inside List.fold_left (fun ll l -> ll ^ "_" ^ l) ""

Copy link
Member Author

Choose a reason for hiding this comment

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

If it is double, then module type name would be ___Belt_List__

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great thanks.

cristianoc
cristianoc approved these changes Apr 22, 2023
View reviewed changes
Copy link
Collaborator

@cristianoc cristianoc left a comment

Choose a reason for hiding this comment

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

If the tests are fine this is ready to merge I think.

@mununki
Copy link
Member Author

mununki commented Apr 22, 2023

If the tests are fine this is ready to merge I think.

Thanks!
I'm still looking into why those tests are not passing.

@mununki mununki force-pushed the more-dynamic-import branch from 713b3b9 to e70407a Compare April 23, 2023 02:38
@mununki mununki force-pushed the more-dynamic-import branch from e70407a to 13af346 Compare April 23, 2023 08:40
@mununki mununki merged commit 6f0c22a into import-mwk Apr 23, 2023
@mununki mununki deleted the more-dynamic-import branch April 23, 2023 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cristianoc cristianoc cristianoc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mununki @cristianoc