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

Replace flushDiscreteUpdates with flushSync #21775

Merged

Commits on Jun 30, 2021

  1. Replace flushDiscreteUpdates with flushSync

    flushDiscreteUpdates is almost the same as flushSync. It forces passive
    effects to fire, because of an outdated heuristic, which isn't ideal but
    not that important.
    
    Besides that, the only remaining difference between flushDiscreteUpdates
    and flushSync is that flushDiscreteUpdates does not warn if you call it
    from inside an effect/lifecycle. This is because it might get triggered
    by a nested event dispatch, like `el.focus()`.
    
    So I added a new method, flushSyncWithWarningIfAlreadyRendering, which
    is used for the public flushSync API. It includes the warning. And I
    removed the warning from flushSync, so the event system can call that
    one. In production, flushSyncWithWarningIfAlreadyRendering gets inlined
    to flushSync, so the behavior is identical.
    
    Another way of thinking about this PR is that I renamed flushSync to
    flushSyncWithWarningIfAlreadyRendering and flushDiscreteUpdates to
    flushSync (and fixed the passive effects thing). The point is to prevent
    these from subtly diverging in the future.
    acdlite committed Jun 30, 2021
    Configuration menu
    Copy the full SHA
    67e8bc8 View commit details
    Browse the repository at this point in the history
  2. Invert so the one with the warning is the default one

    To make Seb happy
    acdlite committed Jun 30, 2021
    Configuration menu
    Copy the full SHA
    464ee05 View commit details
    Browse the repository at this point in the history