chore(TransactionFeedV2): Persist and rehydrate the first page of the feed #6157
Conversation
sviderock
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## slava/feed-v2-track-cross-chain #6157 +/- ##
=================================================================
Coverage 88.83% 88.84%
=================================================================
Files 728 728
Lines 30899 30916 +17
Branches 5665 5366 -299
=================================================================
+ Hits 27449 27466 +17
- Misses 3254 3408 +154
+ Partials 196 42 -154
... and 66 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
| useEffect( | ||
| function updatePersistedFeedFirstPage() { | ||
| const isFirstPage = originalArgs?.endCursor === FIRST_PAGE_TIMESTAMP | ||
|
|
||
| if (isFirstPage && data?.transactions) { | ||
| dispatch(updateFeedFirstPage(data.transactions)) | ||
| } | ||
| }, | ||
| [data?.transactions, originalArgs?.endCursor] | ||
| ) |
There was a problem hiding this comment.
It seems we could skip this effect by reacting to the underlying RTK-Query API fetched action, within the transactions reducer, right?
There was a problem hiding this comment.
@jeanregisser I'll investigate this! RTK-Query dispatches transactionFeedV2Api/executeQuery/fulfilled action whenever the data fetch promise is successfully fulfilled but I'll need to look if there's a type-safe way of catching this action rather than:
const TYPESAFE_FEED_DATA_RECEIVED_ACTION = "transactionFeedV2Api/executeQuery/fulfilled"
switch (action.type) {
case TYPESAFE_FEED_DATA_RECEIVED_ACTION:
return { ...state, ... }
}But if there's no type-safe way - do you think this ^ approach would be sufficient enought?
There was a problem hiding this comment.
It seems like there is a type-safe way but it requires the use of createSlice. Then we can use the extraReducers:
extraReducers: (builder) => {
builder.addMatcher(
transactionFeedV2Api.endpoints.login.matchFulfilled,
(state, { payload }) => {
// modify the state
},
)
},But for this we'll need to re-write the existing transactions/reducer.ts to slice.
There was a problem hiding this comment.
Thanks, moving it to to a slice would be good then.
src/transactions/api.ts
Outdated
| extractRehydrationInfo: (action): any => { | ||
| if (isRehydrateAction(action)) { | ||
| const persistedWeb3 = getRehydratePayload(action, 'web3') as RootState['web3'] | undefined | ||
| const walletAddress = persistedWeb3?.account?.toLowerCase() ?? null | ||
|
|
||
| const persistedTransactions = getRehydratePayload(action, 'transactions') as | ||
| | RootState['transactions'] | ||
| | undefined | ||
| const feedFirstPage = persistedTransactions?.feedFirstPage || [] | ||
|
|
||
| if (walletAddress && feedFirstPage.length) { | ||
| const queryKey = `transactionFeedV2({"address":"${walletAddress}","endCursor":${FIRST_PAGE_TIMESTAMP}})` | ||
| return { | ||
| mutations: {}, | ||
| provided: {}, | ||
| subscriptions: {}, | ||
| queries: { | ||
| [queryKey]: { | ||
| status: QueryStatus.fulfilled, | ||
| endpointName: 'transactionFeedV2', | ||
| requestId: nanoid(), | ||
| originalArgs: { address: walletAddress, endCursor: FIRST_PAGE_TIMESTAMP }, | ||
| startedTimeStamp: Date.now(), | ||
| data: { transactions: feedFirstPage }, | ||
| fulfilledTimeStamp: Date.now(), | ||
| error: undefined, | ||
| }, | ||
| }, | ||
| config: { | ||
| online: true, | ||
| focused: true, | ||
| middlewareRegistered: true, | ||
| refetchOnFocus: false, | ||
| refetchOnReconnect: false, | ||
| refetchOnMountOrArgChange: false, | ||
| keepUnusedDataFor: 60, | ||
| reducerPath: 'transactionFeedV2Api', | ||
| invalidationBehavior: 'delayed', | ||
| }, | ||
| } satisfies RootState['transactionFeedV2Api'] |
There was a problem hiding this comment.
I guess this works, but it feels like it's quite specific to underlying details of RTK-Query.
And could break easily with updates.
What do you think of doing something like const currentData = data.transactions || feedFirstPage at the component instead?
There was a problem hiding this comment.
@jeanregisser
When RTK-Query fetches the data - it first casts it to undefined and after the request is finished populates it with the data from the response. This means that on every page load there will always be an extra re-render where data?.transactions || feedFirstPage will evaluate to feedFirstPage as data would be still undefined for the time of fetching. And this will subsequently trigger the merge of every page with the feedFirstPage.
I'm sure there is a solution that will work that might be similar to what you've proposed (maybe with the init ref flag and useEffect) but this might introduce more complexity as there's gonna be a new issue of distinguishing between initial first page update and subsequent polling updates.
If there's gonna be an update to the library via renovate - the satisfies RootState['transactionFeedV2Api'] clause will ensure that the errors will be thrown if the underlying structure changes which would force us to update our code alongside the library update.
But the caveat is real indeed, as this creates an extra point of maintenance. Although, from the info I've gathered from the Github issues of RTK-Query related specifically to usage with persisted storage and offline usage - this seems to be the recommended approach by the RTK-Query team themselves. I put the link to one of the issues in the comment above the implementation as it might provide a bit more context.
Description
10th PR of RET-1207. Adds a new
feedFirstPagefield to the Redux so the first page of the feed can be persisted and rehydrated on the app init.Test plan
Added test to check a proper rehydration flow for the first page of the feed
Related issues
Backwards compatibility
Yes
Network scalability
If a new NetworkId and/or Network are added in the future, the changes in this PR will: