Add reusable component for screens that need server data.#4601
Closed
chrisbobbe wants to merge 9 commits intozulip:masterfrom
Closed
Add reusable component for screens that need server data.#4601chrisbobbe wants to merge 9 commits intozulip:masterfrom
chrisbobbe wants to merge 9 commits intozulip:masterfrom
Conversation
In our understanding of the reason to use `connect` (see the long code comment above the `connect` call), it shouldn't be necessary to use `connect` to grab any particular value; the essential thing is that the component be wrapped with a `connect` call. So, keep doing that, but use `useSelector` to get `haveServerData`.
I always forget the order in which the functions are applied, but I think this is correct. From the doc [1]: """ (arguments): The functions to compose. Each function is expected to accept a single parameter. Its return value will be provided as an argument to the function standing to the left, and so on. The exception is the right-most argument which can accept multiple parameters, as it will provide the signature for the resulting composed function. """ [1] https://redux.js.org/api/compose
Combining the `connect` call and the show-if-server-data conditional into one reusable HOC.
It might be nice for `withHaveServerDataGate` to be usable without
having to think about a `dispatch` prop, i.e., if we can think of
the `connect` call as a boring implementation detail. So I've done
the following cast, meaning to fudge over the fact that the
`connect`ed component has access to `dispatch`:
```diff
- connect<{||}, _, _>(),
+ (connect<{||}, _, _>(): (ComponentType<P>) => ComponentType<P>),
```
I expected to have to do a `$FlowIgnore` right there, on that cast.
Instead, I'm seeing two Flow errors *elsewhere*, and they look like
they're on the path to `connect`'s annotation:
```
Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ src/generics.js:44:3
Cannot create exact type from P [1]. [invalid-exact]
41│ export type BoundedDiff<-U, -L> = $Diff<
42│ // This `IsSupertype` is the part that checks that all of L's properties
43│ // (a) are present on U and (b) have appropriate types to use on U.
[1] 44│ IsSupertype<U, $ReadOnly<{| ...U, ...L |}>>,
45│ // This `$ObjMap` makes a variant of `L` that `$Diff` treats as more
46│ // powerful, ensuring that all properties in `L` are removed completely.
47│ $ObjMap<L, () => mixed>,
Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ src/react-redux.js:15:22
Cannot create exact type from P [1]. [invalid-exact]
12│ /* eslint-disable flowtype/generic-spacing */
13│
14│ export type OwnProps<-C, -SP> = $Diff<
[1] 15│ BoundedDiff<$Exact<ElementConfig<C>>, SP>,
16│ {| dispatch: Dispatch |},
17│ >;
18│
```
- Do you see these errors too?
- Do we actually want to fudge over `dispatch` like this?
- It sounds like there might be a Flow bug...
- But also, could there be something wrong with `connect`'s
annotation?
Contributor
Author
|
Resumed in #4603. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
As discussed here. The intention is to use this for
ChatScreenwhich needs something like this, with at least P1 urgency; see discussion.Marked as a draft because there are some Flow errors that I haven't managed to work through yet; I'd appreciate if @gnprice would take a look. 🙂