-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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 #2516
Dynamic import #2516
Conversation
The core point of the fix is here: https://github.com/denoland/deno/pull/2516/files#diff-7170d073ffb6b76573055a7aaba20d7fR568 |
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.
Does this compile for you? I'm getting type errors.
I kinda see the point of having this register()
function to pass the &mut Isolate
into the module code, but I don't understand what is being streamed.
43fd625
to
01b98de
Compare
@@ -140,6 +161,24 @@ impl<L: Loader> RecursiveLoad<L> { | |||
|
|||
Ok(module_name) | |||
} | |||
|
|||
pub fn get_future( |
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.
Eventually this should go. I added it to make porting over the tests easier.
ref #1789 |
e5bb620
to
7b85cb9
Compare
8fd5c92
to
3449974
Compare
Update: This is close to landing and still being worked on. |
e9b19e5
to
39d6eb6
Compare
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.
Some remarks from me. I need a bit more time to grok modules::State
though...
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.
/core/modules.rs
became very hard to understand again, but we can surely update it down the road. It's awesome to finally have dynamic imports!
On appveyor the unit tests are failing. No idea why.
|
It's disabled https://github.com/denoland/deno/pull/2516/files#diff-06291984cb0c2f3e0fad6822ab277aec and not part of the unit tests. I think we can land without error_014_catch_dynamic_import_error.js working yet. |
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.
LGTM
583467e
to
0a56326
Compare
0a56326
to
6fbf2e9
Compare
My attempt at fixing #2510.
The scope went a little overboard.
cc @ry