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

Improve internet-connectivity state a bit; upgrade @react-native-community/netinfo. #4940

Merged
merged 10 commits into from
Aug 27, 2021

Commits on Aug 27, 2021

  1. netinfo libdef: Add and use, instead of writing types in AppEventHand…

    …lers.
    
    The version we're on now (5.9.10) is much later than 3.2.1, which it
    looks like was what informed the types written down in
    AppEventHandlers.
    
    One nice-looking thing we get is `isInternetReachable`, which should
    actually tell us whether the Internet is reachable (not just whether
    there's an active network connection).
    chrisbobbe authored and gnprice committed Aug 27, 2021
    Configuration menu
    Copy the full SHA
    0df08eb View commit details
    Browse the repository at this point in the history
  2. deps: Update @react-native-community/netinfo to 6.0.0, the latest.

    And update the libdef by transferring over the changes made in the
    two TypeScript files it's based on.
    
    This handles the only breaking change announced:
    
    > - useNetinfo: When the connection state is unknown, the
    >   `isConnected` and `isInternetReachable` properties are now set
    >   to `null` rather than `false`. This allow you to easily detect
    >   the initial "unknown" state before the state is detected and set
    >   to a boolean.
    chrisbobbe authored and gnprice committed Aug 27, 2021
    Configuration menu
    Copy the full SHA
    4bc0c61 View commit details
    Browse the repository at this point in the history
  3. online state: Switch to isInternetReachable, from connectionType.

    This is hopefully more accurate about what we actually want to know:
    "can the app expect to reach the Internet?" Not just "is there some
    network connection?".
    
    Getting this right is already important for a nice user experience.
    But it'll be more critical if we adapt our request retry-and-timeout
    logic to be aware of the Internet reachability state, which is
    reasonable to want to do.
    
    In 6.0.0, `isInternetReachable` started being `boolean | null`; see
    the comment about when it's `null`.
    
    Since `null` is a meaningful third state, adjust the Redux plumbing
    to allow `null` for `state.session.isOnline` so we can do something
    distinct in the UI.
    chrisbobbe authored and gnprice committed Aug 27, 2021
    Configuration menu
    Copy the full SHA
    e8ef63e View commit details
    Browse the repository at this point in the history
  4. usePrevious [nfc]: Default to null on first render.

    As Greg pointed out a few weeks ago [1], `usePrevious` has had a
    "funny quirk where on the first call, it takes the "previous" value
    to be the current one."
    
    Instead, let the caller decide what the value should be on the first
    render, or just have it be `null` if the caller is fine with that.
    
    [1] zulip#4914 (comment)
    chrisbobbe authored and gnprice committed Aug 27, 2021
    Configuration menu
    Copy the full SHA
    4957259 View commit details
    Browse the repository at this point in the history
  5. usePrevious [nfc]: Tweak jsdoc after recent change.

    Mostly to get the summary line back down to a line, pushing a
    detail out into the prose body.
    
    Also update the reference to the upstream source we based this on,
    now that we've improved it in a way that changes its interface.
    gnprice committed Aug 27, 2021
    Configuration menu
    Copy the full SHA
    be0dda5 View commit details
    Browse the repository at this point in the history
  6. usePrevious [nfc]: Default to undefined instead of null.

    The main motivation for this is that it makes the types behave more
    uniformly between the case where `initValue` is or isn't provided:
    this way, the value we actually use for it is always the value passed,
    rather than sometimes being that and sometimes of a different type.
    
    That in turn will let us tighten the types so that when we have a
    real initial value to provide, the return type becomes simply `T`
    rather than `T | null` or `T | void`.
    gnprice committed Aug 27, 2021
    Configuration menu
    Copy the full SHA
    e033a06 View commit details
    Browse the repository at this point in the history
  7. usePrevious [nfc]: Let types support a non-nullish initValue.

    This way, in cases where we have an initial value that's actually of
    the main value's type `T` (like false or zero), we can see that the
    return value is always of type `T`, rather than have the type-checker
    think that it might be null or undefined.
    gnprice committed Aug 27, 2021
    Configuration menu
    Copy the full SHA
    646cdde View commit details
    Browse the repository at this point in the history
  8. Configuration menu
    Copy the full SHA
    da59c2b View commit details
    Browse the repository at this point in the history
  9. Configuration menu
    Copy the full SHA
    5a5b0de View commit details
    Browse the repository at this point in the history
  10. OfflineNotice: Show an alert if we stay in an unknown state too long.

    And log what we know from NetInfo to Sentry.
    chrisbobbe authored and gnprice committed Aug 27, 2021
    Configuration menu
    Copy the full SHA
    88b0a71 View commit details
    Browse the repository at this point in the history