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

Delete flushSuspenseFallbacksInTests flag #18596

Merged
merged 2 commits into from
Apr 14, 2020

Commits on Apr 13, 2020

  1. Configuration menu
    Copy the full SHA
    c2c2cf5 View commit details
    Browse the repository at this point in the history

Commits on Apr 14, 2020

  1. Delete flushSuspenseFallbacksInTests

    This was meant to be a temporary hack to unblock the `act` work, but it
    quickly spread throughout our tests.
    
    What it's meant to do is force fallbacks to flush inside `act` even in
    Concurrent Mode. It does this by wrapping the `setTimeout` call in a
    check to see if it's in an `act` context. If so, it skips the delay and
    immediately commits the fallback.
    
    Really this is only meant for our internal React tests that need to
    incrementally render. Nobody outside our team (and Relay) needs to do
    that, yet. Even if/when we do support that, it may or may not be with
    the same `flushAndYield` pattern we use internally.
    
    However, even for our internal purposes, the behavior isn't right
    because a really common reason we flush work incrementally is to make
    assertions on the "suspended" state, before the fallback has committed.
    There's no way to do that from inside `act` with the behavior of this
    flag, because it causes the fallback to immediately commit. This has led
    us to *not* use `act` in a lot of our tests, or to write code that
    doesn't match what would actually happen in a real environment.
    
    What we really want is for the fallbacks to be flushed at the *end` of
    the `act` scope. Not within it.
    
    This only affects the noop and test renderer versions of `act`, which
    are implemented inside the reconciler. Whereas `ReactTestUtils.act` is
    implemented in "userspace" for backwards compatibility. This is fine
    because we didn't have any DOM Suspense tests that relied on this flag;
    they all use test renderer or noop.
    
    In the future, we'll probably want to move always use the reconciler
    implementation of `act`. It will not affect the prod bundle, because we
    currently only plan to support `act` in dev. Though we still haven't
    completely figured that out. However, regardless of whether we support a
    production `act` for users, we'll still need to write internal React
    tests in production mode. For that use case, we'll likely add our own
    internal version of `act` that assumes a mock Scheduler and might rely
    on hacks that don't 100% align up with the public one.
    acdlite committed Apr 14, 2020
    Configuration menu
    Copy the full SHA
    3a12db4 View commit details
    Browse the repository at this point in the history