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

Fix allowing using dyanmic import of module instead of async function #6434

Merged
merged 4 commits into from
Oct 17, 2023

Conversation

mununki
Copy link
Member

@mununki mununki commented Oct 11, 2023

This PR fixes not allowing to using the dynamic import of module in the normal function that is not async function.

// should not compiled
let f0 = () => {
  module O = await Belt.Option
  O.forEach
}

// Ok
let f1 = async () => {
  module O = await Belt.Option
  O.forEach
}

Plus, This PR fixes build error using the dynamic import of module in uncurried mode:

// should be fine in uncurried mode
let f1 = async () => {
  module O = await Belt.Option
  O.forEach
}

@mununki
Copy link
Member Author

mununki commented Oct 12, 2023

Ready to get a review

@zth zth requested a review from cristianoc October 12, 2023 07:46
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.

Add a test maybe?

@mununki
Copy link
Member Author

mununki commented Oct 12, 2023

Import.res has it already. After this PR is merged, if missing async will break the test.

@mununki mununki force-pushed the async-context-await-module branch from 9937e8a to a0c1f45 Compare October 12, 2023 12:05
@mununki mununki requested a review from cristianoc October 17, 2023 12:21
@mununki mununki merged commit 330b256 into master Oct 17, 2023
14 checks passed
@mununki mununki deleted the async-context-await-module branch October 19, 2023 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants