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

refactor(core): JsRuntime is not a Future #7855

Merged
merged 6 commits into from
Oct 7, 2020

Conversation

bartlomieju
Copy link
Member

This commit rewrites deno_core::JsRuntime to not implement Future
trait.

Instead there are two separate methods:

  • JsRuntime::poll_event_loop() - does single tick of event loop
  • JsRuntime::run_event_loop() - runs event loop to completion

This commit rewrites deno_core::JsRuntime to not implement Future
trait.

Instead there are two separate methods:
- JsRuntime::poll_event_loop() - does single tick of event loop
- JsRuntime::run_event_loop() - runs event loop to completion
Comment on lines +457 to +464
/// Runs event loop to completion
///
/// This future resolves when:
/// - there are no more pending dynamic imports
/// - there are no more pending ops
pub async fn run_event_loop(&mut self) -> Result<(), AnyError> {
poll_fn(|cx| self.poll_event_loop(cx)).await
}
Copy link
Member Author

Choose a reason for hiding this comment

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

@ry I still prefer run_event_loop - are you ok with it?

Comment on lines +12 to +17
a way to execute JavaScript.

The JsRuntime implements an event loop abstraction for the executed code that
keeps track of all pending tasks (async ops, dynamic module loads). It is user's
responsibility to drive that loop by using `JsRuntime::run_event_loop` method -
it must be executed in the context of Rust's future executor (eg. tokio, smol).
Copy link
Member Author

Choose a reason for hiding this comment

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

@ry

Copy link
Member

Choose a reason for hiding this comment

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

LGTM - land it

@bartlomieju bartlomieju merged commit d8879fe into denoland:master Oct 7, 2020
@bartlomieju bartlomieju deleted the refactor_runtime_future branch October 7, 2020 20:30
ry added a commit to ry/deno that referenced this pull request Oct 10, 2020
bartlomieju pushed a commit that referenced this pull request Oct 10, 2020
* Revert "refactor: Worker is not a Future (#7895)"

This reverts commit f4357f0.

* Revert "refactor(core): JsRuntime is not a Future (#7855)"

This reverts commit d8879fe.

* Revert "fix(core): module execution with top level await (#7672)"

This reverts commit c7c7677.
iykekings pushed a commit to iykekings/deno that referenced this pull request Oct 10, 2020
* Revert "refactor: Worker is not a Future (denoland#7895)"

This reverts commit f4357f0.

* Revert "refactor(core): JsRuntime is not a Future (denoland#7855)"

This reverts commit d8879fe.

* Revert "fix(core): module execution with top level await (denoland#7672)"

This reverts commit c7c7677.
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.

3 participants