redux: Various small prep changes for multi-org support #5016
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
This doesn't have any callers.
This thunk action reportPresence is in much the same situation as most of our other thunk actions: it's meant to act on a single account's data; it acts on the active account's data; it has a bug where if the active account changes while its network request is pending, it can apply the response to the wrong account's data; and it can't do much if there is no logged-in active account. It had been unusual in that it checked at the top for a logged-in active account, and bailed out silently if there was none. That's because it's unusual in that it's called from a spot that starts operating before the user logs in, and keeps operating if they log out. But ever since we overhauled the presence-reporting code in 6efc449, that caller (PresenceHeartbeat) already checks whether there's a logged-in active account, and only invokes this action if there is. So it can go ahead and make that assumption exactly the same way that the bulk of our thunk actions do. That will simplify bringing it along with the rest when we make them act on a specific account they're dispatched for. Also add a couple of related notes for that future migration.
All of these are in per-account contexts: the main UI, or a server fetch (indeed the initial fetch.) None of them need the whole Account object. That object includes the API key, which is a good reason not to pass it around where we can avoid it; and it has both the user's identity on the server and miscellaneous bits like the server version, which might get rearranged to separate places in the future, so it's convenient to put those through separate indirections like getIdentity and getServerVersion.
This is really just an internal helper for sendOutbox -- the body of its retry loop.
I don't really know what this parameter means, so can't evaluate if false is the right value for it. The history doesn't seem to be enlightening; we've had it this way since early on. Also make the other argument `isActive` non-optional. The existing call site passes a value for it explicitly, and this will keep it that way.
These are all module-private helpers that get these parameter's types inferred from their callers. (Those callers ultimately get the types via ThunkAction.) We'll be changing the definition of ThunkAction, and it's helpful to have fewer redundant references that need to be updated at the same time.
We've just eliminated all the other references to this. We used to have them all over our thunk actions. We recently switched to annotating those with ThunkAction, and we've just gone and eliminated the remaining references, which were on thunk-action-like helpers.
This module hasn't needed a Redux `dispatch` since 543c07c, when we stopped using Redux for dispatching navigation actions. So, simplify by not taking one in the first place.
We haven't needed this since we upgraded off of react-navigation v1, in 98836b8 two years ago.
These are all bags of React props, so they should indeed be read-only object types.
chrisbobbe
approved these changes
Sep 21, 2021
| unsubs: Array<() => void> = []; | ||
|
|
||
| constructor(dispatch: Dispatch) { | ||
| this.dispatch = dispatch; |
There was a problem hiding this comment.
sharing [nfc]: Stop taking dispatch, no longer used here
This module hasn't needed a Redux `dispatch` since 543c07c, when
we stopped using Redux for dispatching navigation actions. So,
simplify by not taking one in the first place.
This is great!!
| @@ -384,13 +384,11 @@ type NonMaybeGlobalState = NonMaybeProperties<GlobalState>; | |||
| // the corresponding parameter when used in `createSelector`, and this does. | |||
| export type Selector<TResult, TParam = void> = InputSelector<GlobalState, TParam, TResult>; | |||
|
|
|||
| export type GetState = () => GlobalState; | |||
chrisbobbe
force-pushed
the
pr-multi-account-prep
branch
from
1d858d0
to
e19f9cd
Compare
This was referenced Sep 22, 2021
This was referenced Oct 21, 2021
This was referenced Nov 5, 2021
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.
This is the first PR in a series produced from the branch described at #5006 (comment) .
This first PR doesn't yet get into the main substance of the changes. It collects a variety of small, mostly independent, prep commits needed along the way, that were quickest to polish into a PR and should be generally easy to review.