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.

Sign up for GitHub

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

Jump to bottom

Anticipate and fix a few Flow errors we'd get with RN v0.63 and Flow 0.122. #4414

Merged
merged 6 commits into from
Jan 14, 2021
Merged

Anticipate and fix a few Flow errors we'd get with RN v0.63 and Flow 0.122. #4414

merged 6 commits into from
Jan 14, 2021

Commits on Jan 14, 2021

  1. notification tests types: Import Notification. 

    A built-in type called `Notification` [1] was being used here; we
should use our custom-defined one.

[1] https://github.com/facebook/flow/blob/v0.113.0/lib/dom.js#L4030
    @chrisbobbe
    chrisbobbe committed Jan 14, 2021
    Configuration menu
    Copy the full SHA
    dcce637 View commit details
    Browse the repository at this point in the history

  2. AnimatedRotateComponent [nfc]: Add simple type annotation. 

    With the new Flow version (0.122.0) coming along with the React
Native v0.63 upgrade (zulip#4245), this would be flagged with a "Could
not decide which case to select" error.

I'm not sure why it has trouble distinguishing this array of strings
from a possible array of numbers, but it's simple enough to just
annotate it as an array of strings, so, might as well.
    @chrisbobbe
    chrisbobbe committed Jan 14, 2021
    Configuration menu
    Copy the full SHA
    3a9c01f View commit details
    Browse the repository at this point in the history

  3. SlideAnimationView [nfc]: Add simple type annotation. 

    Similar to what we did in the previous commit.
    @chrisbobbe
    chrisbobbe committed Jan 14, 2021
    Configuration menu
    Copy the full SHA
    4b38915 View commit details
    Browse the repository at this point in the history

  4. TranslationProvider [nfc]: Annotate TranslationContext. 

    Despite passing `undefined` as the `defaultValue` argument to
`React.createContext`, consumers will never see `undefined` as the
value for `this.context` or `useContext(TranslationContext)`.

That is, as long as `TranslationContext.Provider` is above such
consumers in the component hierarchy. From React's doc on
`createContext` [1],

"""
The `defaultValue` argument is **only** used when a component does
not have a matching Provider above it in the tree. This can be
helpful for testing components in isolation without wrapping them.
"""

Adding this annotation will satisfy the new version of Flow
(0.122.0) that we'll get along with the React Native v0.63 upgrade
(zulip#4245) soon.

[1] https://reactjs.org/docs/context.html#reactcreatecontext
    @chrisbobbe
    chrisbobbe committed Jan 14, 2021
    Configuration menu
    Copy the full SHA
    4249682 View commit details
    Browse the repository at this point in the history

  5. RealmScreen [nfc]: Remove annotation for instance property we don't use.

    @chrisbobbe
    chrisbobbe committed Jan 14, 2021
    Configuration menu
    Copy the full SHA
    ac64b4e View commit details
    Browse the repository at this point in the history

  6. messagesReducer [nfc]: Fix a sketchy null check. 

    This causes Flow to mark the `$FlowFixMe` on
`EventUpdateMessageAction.orig_subject?` as an "unused suppression
comment", so, remove it. In fact, I don't think a `$FlowFixMe`
really belongs here; there's no question that `orig_subject` is
sometimes missing, if only because private messages don't have
topics. The problem is in working out when it's there and when it's
not.

As I point out in the ongoing discussion [1] linked from that TODO,
there's a particular line [2] in the server code at current `master`
that I believe ensures that `update_message` events won't come to us
with an empty string for `orig_subject`.

[1] https://chat.zulip.org/#narrow/stream/206-zulip-terminal/topic/subject.20always.20present.20in.20event/near/1098954
[2] https://github.com/zulip/zulip/blob/08d716c74175a8b63bf8eb1080381c77f3f853c6/zerver/views/message_edit.py#L157
    @chrisbobbe
    chrisbobbe committed Jan 14, 2021
    Configuration menu
    Copy the full SHA
    e221fc1 View commit details
    Browse the repository at this point in the history