Anticipate and fix a few Flow errors we'd get with RN v0.63 and Flow 0.122. #4414
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chrisbobbe
force-pushed
the
pr-fix-future-flow-errors
branch
from
b66480d
to
aebf99b
Compare
gnprice
reviewed
Jan 14, 2021
src/common/SlideAnimationView.js
Outdated
| @@ -46,7 +46,7 @@ export default class SlideAnimationView extends PureComponent<Props, State> { | |||
| const { property, from, to, movement, style } = this.props; | |||
| const animationValue = this.state.animationIndex.interpolate({ | |||
| inputRange: [0, 1], | |||
| outputRange: movement === 'out' ? [from, to] : [to, from], | |||
| outputRange: movement === 'out' ? ([from, to]: number[]) : ([to, from]: number[]), | |||
There was a problem hiding this comment.
Could this annotation be applied once to the whole expression? Would make it a bit shorter.
There was a problem hiding this comment.
Yep! Thanks, done.
chrisbobbe
force-pushed
the
pr-fix-future-flow-errors
branch
from
aebf99b
to
f3b2805
Compare
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
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.
Similar to what we did in the previous commit.
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
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
force-pushed
the
pr-fix-future-flow-errors
branch
from
f3b2805
to
e221fc1
Compare
|
Thanks for the review! Merged. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On #4245, the issue for upgrading to React Native v0.63, I said:
Well, #4296 has been resolved! As we'd hoped, the new Flow version (0.122.0) doesn't complain about anything that has to do with react-navigation anymore, which brings us closer to being able to take the Flow version along with RN v0.63.
This branch whittles down the new Flow version's output to the single warning below, with the helpful comment that we can remove a $FlowFixMe along with the upgrade.