-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: add scheduler::flush() to replace sleep(Duration::ZERO) in tests #4044
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,34 @@ | |
| use std::cell::RefCell; | ||
| use std::collections::BTreeMap; | ||
| use std::rc::Rc; | ||
| #[cfg(any(test, feature = "test"))] | ||
| mod flush_wakers { | ||
| use std::cell::RefCell; | ||
| use std::task::Waker; | ||
|
|
||
| thread_local! { | ||
| static FLUSH_WAKERS: RefCell<Vec<Waker>> = Default::default(); | ||
| } | ||
|
|
||
| #[cfg(all( | ||
| target_arch = "wasm32", | ||
| not(target_os = "wasi"), | ||
| not(feature = "not_browser_env") | ||
| ))] | ||
| pub(super) fn register(waker: Waker) { | ||
| FLUSH_WAKERS.with(|w| { | ||
| w.borrow_mut().push(waker); | ||
| }); | ||
| } | ||
|
|
||
| pub(super) fn wake_all() { | ||
| FLUSH_WAKERS.with(|w| { | ||
| for waker in w.borrow_mut().drain(..) { | ||
| waker.wake(); | ||
| } | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| /// Alias for `Rc<RefCell<T>>` | ||
| pub type Shared<T> = Rc<RefCell<T>>; | ||
|
|
@@ -207,6 +235,8 @@ pub(crate) fn start_now() { | |
| LOCK.with(|l| { | ||
| if let Ok(_lock) = l.try_borrow_mut() { | ||
| scheduler_loop(); | ||
| #[cfg(any(test, feature = "test"))] | ||
| flush_wakers::wake_all(); | ||
| } | ||
| }); | ||
| } | ||
|
|
@@ -232,6 +262,11 @@ mod arch { | |
| IS_SCHEDULED.store(is, Ordering::Relaxed) | ||
| } | ||
|
|
||
| #[cfg(any(test, feature = "test"))] | ||
| pub(super) fn is_scheduled() -> bool { | ||
| check_scheduled() | ||
| } | ||
|
|
||
| /// We delay the start of the scheduler to the end of the micro task queue. | ||
| /// So any messages that needs to be queued can be queued. | ||
| pub(crate) fn start() { | ||
|
|
@@ -260,10 +295,94 @@ mod arch { | |
| pub(crate) fn start() { | ||
| super::start_now(); | ||
| } | ||
|
|
||
| #[cfg(any(test, feature = "test"))] | ||
| #[allow(dead_code)] | ||
| pub(super) fn is_scheduled() -> bool { | ||
| false | ||
| } | ||
| } | ||
|
|
||
| pub(crate) use arch::*; | ||
|
|
||
| /// Flush all pending scheduler work, ensuring all rendering and lifecycle callbacks complete. | ||
| /// | ||
| /// On browser WebAssembly targets, the scheduler defers its work to the microtask queue. | ||
| /// This function registers a waker that is notified when `start_now()` finishes draining all | ||
| /// queues, providing proper event-driven render-complete notification without arbitrary sleeps. | ||
| /// | ||
| /// On non-browser targets, the scheduler runs synchronously so this simply drains pending work. | ||
| /// | ||
| /// Use this in tests after mounting or updating a component to ensure all rendering has | ||
| /// completed before making assertions. | ||
| #[cfg(all( | ||
| any(test, feature = "test"), | ||
| target_arch = "wasm32", | ||
| not(target_os = "wasi"), | ||
| not(feature = "not_browser_env") | ||
| ))] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could these three lines be moved into the function for a uniform signature (and documentation) across targets.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done, both targets now use |
||
| pub fn flush() -> Flush { | ||
| Flush { _priv: () } | ||
| } | ||
|
|
||
| /// Future returned by [`flush()`] that resolves when the scheduler finishes all pending work. | ||
| /// | ||
| /// On each poll, this future eagerly drains any currently-queued scheduler work via | ||
| /// `start_now()`. It then checks whether the scheduler has been re-scheduled (via | ||
| /// `IS_SCHEDULED`), which indicates that spawned microtasks (e.g., from Suspense futures | ||
| /// resolving) will trigger more work. If so, it re-registers its waker and yields, allowing | ||
| /// those microtasks to execute before the next poll. This loop continues until the scheduler | ||
| /// is truly idle. | ||
| #[cfg(all( | ||
| any(test, feature = "test"), | ||
| target_arch = "wasm32", | ||
| not(target_os = "wasi"), | ||
| not(feature = "not_browser_env") | ||
| ))] | ||
| #[derive(Debug)] | ||
| pub struct Flush { | ||
| _priv: (), | ||
| } | ||
|
|
||
| #[cfg(all( | ||
| any(test, feature = "test"), | ||
| target_arch = "wasm32", | ||
| not(target_os = "wasi"), | ||
| not(feature = "not_browser_env") | ||
| ))] | ||
| impl std::future::Future for Flush { | ||
| type Output = (); | ||
|
|
||
| fn poll( | ||
| self: std::pin::Pin<&mut Self>, | ||
| cx: &mut std::task::Context<'_>, | ||
| ) -> std::task::Poll<()> { | ||
| start_now(); | ||
|
|
||
| if arch::is_scheduled() { | ||
| flush_wakers::register(cx.waker().clone()); | ||
| std::task::Poll::Pending | ||
| } else { | ||
| std::task::Poll::Ready(()) | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This future could be simply implemented with
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call, done. |
||
| /// Flush all pending scheduler work, ensuring all rendering and lifecycle callbacks complete. | ||
| /// | ||
| /// On non-browser targets, the scheduler runs synchronously so this simply drains pending work. | ||
| #[cfg(all( | ||
| any(test, feature = "test"), | ||
| not(all( | ||
| target_arch = "wasm32", | ||
| not(target_os = "wasi"), | ||
| not(feature = "not_browser_env") | ||
| )) | ||
| ))] | ||
| pub async fn flush() { | ||
| start_now(); | ||
| } | ||
|
|
||
| impl Scheduler { | ||
| /// Fill vector with tasks to be executed according to Runnable type execution priority | ||
| /// | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,7 @@ use yew::platform::time::sleep; | |
| use yew::prelude::*; | ||
| use yew::suspense::{use_future, Suspension, SuspensionResult}; | ||
| use yew::virtual_dom::VNode; | ||
| use yew::{component, Renderer, ServerRenderer}; | ||
| use yew::{component, scheduler, Renderer, ServerRenderer}; | ||
|
|
||
| wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser); | ||
|
|
||
|
|
@@ -62,12 +62,12 @@ async fn hydration_works() { | |
| .unwrap() | ||
| .set_inner_html(&s); | ||
|
|
||
| sleep(Duration::ZERO).await; | ||
| scheduler::flush().await; | ||
|
|
||
| Renderer::<App>::with_root(gloo::utils::document().get_element_by_id("output").unwrap()) | ||
| .hydrate(); | ||
|
|
||
| sleep(Duration::ZERO).await; | ||
| scheduler::flush().await; | ||
|
|
||
| let result = obtain_result_by_id("output"); | ||
|
|
||
|
|
@@ -85,7 +85,7 @@ async fn hydration_works() { | |
| .unwrap() | ||
| .click(); | ||
|
|
||
| sleep(Duration::ZERO).await; | ||
| scheduler::flush().await; | ||
|
|
||
| let result = obtain_result_by_id("output"); | ||
|
|
||
|
|
@@ -237,7 +237,7 @@ async fn hydration_with_suspense() { | |
| .unwrap() | ||
| .set_inner_html(&s); | ||
|
|
||
| sleep(Duration::ZERO).await; | ||
| scheduler::flush().await; | ||
|
|
||
| Renderer::<App>::with_root(gloo::utils::document().get_element_by_id("output").unwrap()) | ||
| .hydrate(); | ||
|
|
@@ -393,7 +393,7 @@ async fn hydration_with_suspense_not_suspended_at_start() { | |
| .unwrap() | ||
| .set_inner_html(&s); | ||
|
|
||
| sleep(Duration::ZERO).await; | ||
| scheduler::flush().await; | ||
|
|
||
| Renderer::<App>::with_root(gloo::utils::document().get_element_by_id("output").unwrap()) | ||
| .hydrate(); | ||
|
|
@@ -524,7 +524,7 @@ async fn hydration_nested_suspense_works() { | |
| .unwrap() | ||
| .set_inner_html(&s); | ||
|
|
||
| sleep(Duration::ZERO).await; | ||
| scheduler::flush().await; | ||
|
|
||
| Renderer::<App>::with_root(gloo::utils::document().get_element_by_id("output").unwrap()) | ||
| .hydrate(); | ||
|
|
@@ -661,12 +661,12 @@ async fn hydration_node_ref_works() { | |
| .unwrap() | ||
| .set_inner_html(&s); | ||
|
|
||
| sleep(Duration::ZERO).await; | ||
| scheduler::flush().await; | ||
|
|
||
| Renderer::<App>::with_root(gloo::utils::document().get_element_by_id("output").unwrap()) | ||
| .hydrate(); | ||
|
|
||
| sleep(Duration::ZERO).await; | ||
| scheduler::flush().await; | ||
|
|
||
| let result = obtain_result_by_id("output"); | ||
| assert_eq!( | ||
|
|
@@ -682,7 +682,7 @@ async fn hydration_node_ref_works() { | |
| .unwrap() | ||
| .click(); | ||
|
|
||
| sleep(Duration::ZERO).await; | ||
| scheduler::flush().await; | ||
|
|
||
| let result = obtain_result_by_id("output"); | ||
| assert_eq!( | ||
|
|
@@ -754,16 +754,13 @@ async fn hydration_list_order_works() { | |
| .unwrap() | ||
| .set_inner_html(&s); | ||
|
|
||
| sleep(Duration::ZERO).await; | ||
| scheduler::flush().await; | ||
|
|
||
| Renderer::<App>::with_root(gloo::utils::document().get_element_by_id("output").unwrap()) | ||
| .hydrate(); | ||
|
|
||
| // Wait until all suspended components becomes revealed. | ||
| sleep(Duration::ZERO).await; | ||
| sleep(Duration::ZERO).await; | ||
| sleep(Duration::ZERO).await; | ||
| sleep(Duration::ZERO).await; | ||
| scheduler::flush().await; | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. real proof of flush correctness |
||
|
|
||
| let result = obtain_result_by_id("output"); | ||
| assert_eq!( | ||
|
|
@@ -837,13 +834,13 @@ async fn hydration_suspense_no_flickering() { | |
| .unwrap() | ||
| .set_inner_html(&s); | ||
|
|
||
| sleep(Duration::ZERO).await; | ||
| scheduler::flush().await; | ||
|
|
||
| Renderer::<App>::with_root(gloo::utils::document().get_element_by_id("output").unwrap()) | ||
| .hydrate(); | ||
|
|
||
| // Wait until all suspended components becomes revealed. | ||
| sleep(Duration::ZERO).await; | ||
| scheduler::flush().await; | ||
|
|
||
| let result = obtain_result_by_id("output"); | ||
| assert_eq!( | ||
|
|
@@ -950,16 +947,13 @@ async fn hydration_order_issue_nested_suspense() { | |
| .unwrap() | ||
| .set_inner_html(&s); | ||
|
|
||
| sleep(Duration::ZERO).await; | ||
| scheduler::flush().await; | ||
|
|
||
| Renderer::<App>::with_root(gloo::utils::document().get_element_by_id("output").unwrap()) | ||
| .hydrate(); | ||
|
|
||
| // Wait until all suspended components becomes revealed. | ||
| sleep(Duration::ZERO).await; | ||
| sleep(Duration::ZERO).await; | ||
| sleep(Duration::ZERO).await; | ||
| sleep(Duration::ZERO).await; | ||
| scheduler::flush().await; | ||
|
|
||
| let result = obtain_result_by_id("output"); | ||
| assert_eq!( | ||
|
|
@@ -1180,7 +1174,7 @@ async fn hydration_with_camelcase_svg_elements() { | |
| .unwrap() | ||
| .set_inner_html(&s); | ||
|
|
||
| sleep(Duration::ZERO).await; | ||
| scheduler::flush().await; | ||
|
|
||
| // Hydrate - this should not panic | ||
| Renderer::<App>::with_root(gloo::utils::document().get_element_by_id("output").unwrap()) | ||
|
|
@@ -1268,15 +1262,12 @@ async fn hydration_suspended_child_does_not_trap_sibling_slot() { | |
| .unwrap() | ||
| .set_inner_html(&s); | ||
|
|
||
| sleep(Duration::ZERO).await; | ||
| scheduler::flush().await; | ||
|
|
||
| Renderer::<App>::with_root(gloo::utils::document().get_element_by_id("output").unwrap()) | ||
| .hydrate(); | ||
|
|
||
| sleep(Duration::ZERO).await; | ||
| sleep(Duration::ZERO).await; | ||
| sleep(Duration::ZERO).await; | ||
| sleep(Duration::ZERO).await; | ||
| scheduler::flush().await; | ||
|
|
||
| let result = obtain_result(); | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.