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

Feedback Wanted — change: run effects after a tick #1680

Merged
merged 5 commits into from
Sep 12, 2023
Merged

Conversation

gbj
Copy link
Collaborator

@gbj gbj commented Sep 8, 2023

The Problem

Currently, effects created with create_effect run

  1. only in the browser
  2. immediately

This means that simple code like this fails silently

#[component]
pub fn App() -> impl IntoView {
    let input = create_node_ref::<html::Input>();
    create_effect(move |_| {
        if let Some(input) = input.get() {
            logging::log!("focusing");
            input.focus();
        }
    });
    view! {
        <input node_ref=input/>
    }
}

This logs focusing but does not actually focus the <input> because it has not yet been mounted to the DOM. The sequence of events goes:

  1. create_node_ref: NodeRef is created
  2. create_effect: creates the effect, and runs once immediately to register its dependencies
  3. <input node_ref=input/>: creates an HtmlInputElement and loads it into the NodeRef
  4. loading the NodeRef triggers the effect to run again, calling input.focus()
  5. the HtmlInputElement is returned and mounted to the DOM

Because 4 happens before 5 — i.e., .focus() is called before the element has actually been mounted — the browser does not focus it.

The Solution

This PR waits to run an effect for the first time until one tick after it's created (determined by queueMicrotask: i.e., it runs it after all other synchronous code finishes running.)

This means the same code now runs in the following order:

  1. create_node_ref: NodeRef is created
  2. create_effect: creates the effect, and queues first run
  3. <input node_ref=input/>: creates an HtmlInputElement and loads it into the NodeRef
  4. loading the NodeRef does nothing, because the effect hasn't run yet so it hasn't registered the dependency
  5. the HtmlInputElement is returned and mounted to the DOM
  6. at the end of the process of mounting the app, the effect runs once, correctly focusing the element

This also means the effect only ends up running once, for what it's worth.

This is a breaking change, as it affects the scheduling of effects. However, it tends to schedule effects so that they are closer to what people already assume: they run after the view has been returned and the component mounted, not immediately (i.e., before it.)

This should fix almost every situation in which the current recommendation is

create_effect(move |_| {
  request_animation_frame(move || {
  });
});

This is how SolidJS runs its effects as well, and it distinguishes between createEffect() (user-created effect, runs on next tick) and createRenderEffect() (used internally in renderer, runs immediately.) We would do the same, using immediate synchronous effects in the renderer and delayed effects as the default for user code.

@gbj gbj marked this pull request as draft September 8, 2023 19:28
@gbj gbj marked this pull request as ready for review September 12, 2023 01:01
@gbj gbj merged commit a317874 into main Sep 12, 2023
55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant