Skip to content
This repository was archived by the owner on Oct 4, 2023. It is now read-only.

[C-2607] Pagination wrapper hooks for audius-query#3375

Merged
amendelsohn merged 5 commits into
mainfrom
audius-query-pagination
May 18, 2023
Merged

[C-2607] Pagination wrapper hooks for audius-query#3375
amendelsohn merged 5 commits into
mainfrom
audius-query-pagination

Conversation

@amendelsohn

@amendelsohn amendelsohn commented May 18, 2023

Copy link
Copy Markdown
Contributor

Description

Integrated usage example to come in separate PR

Adds usePaginatedQuery and useAllPaginatedQuery which wrap an existing audius-query hook and manage pagination args and response automatically using our standard limit, offset, hasMore, loadMore interface

  const {
    data,
    status,
	hasMore,
    loadMore
  } = usePaginatedQuery(
    useMyApiQuery,
    {
      myArg1,
	  myArg2
    },
    PAGE_SIZE

Also adds sdk to the AudiusQueryContext, making it available in endpiont fetch functions

@amendelsohn amendelsohn requested review from a team and dylanjeffers May 18, 2023 00:08
@amendelsohn amendelsohn requested a review from sliptype May 18, 2023 00:10
@audius-infra

audius-infra commented May 18, 2023

Copy link
Copy Markdown
Collaborator

Preview this change https://demo.audius.co/audius-query-pagination

sliptype
sliptype approved these changes May 18, 2023
View reviewed changes

@sliptype sliptype left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome stuff!

Comment thread packages/common/src/api/hooks/usePaginatedQuery.ts Outdated
hasMore:
result.status === Status.IDLE ||
(!result.data && result.status === Status.LOADING) ||
result.data?.length < pageSize

@sliptype sliptype May 18, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be result.data?.length == pageSize? If the data length is less than the pageSize that would indicate there are no more items to be fetched

@amendelsohn amendelsohn May 18, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

omg you're right. It's backwards

Comment thread packages/common/src/api/hooks/usePaginatedQuery.ts
ArgsType extends { limit: number; offset: number },
HookType extends (args: ArgsType) => QueryHookResults<Data[]>
>(
useQueryHook: HookType,

@sliptype sliptype May 18, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still not entirely sure this doesn't break the rules of hooks. If the caller were to change the hook being passed, then hooks would be getting called in a different order

useAllPaginatedQuery(something ? useGetTransactions : useGetTracks, {}, 50)

Not sure if it's actually a problem though

dylanjeffers
dylanjeffers approved these changes May 18, 2023
View reviewed changes

@dylanjeffers dylanjeffers left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wonderful additions

Comment thread packages/common/src/api/createApi.ts
const dispatch = useDispatch()
const key = getKeyFromFetchArgs(fetchArgs)
const queryState = useSelector((state: any) => {
if (!state.api[reducerPath]) {

@dylanjeffers dylanjeffers May 18, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love this addition! so when user reads this it would be in context of app, so maybe @audius/common/src/api/reducers.ts to be extra explicit?

Comment thread packages/web/src/app.tsx
return (
<Provider store={store}>
<AudiusQueryContext.Provider value={{ apiClient }}>
<AudiusQueryContext.Provider

@dylanjeffers dylanjeffers May 18, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love this, as it gets bigger, we can maybe add a provider in a separate file that does all the config

dylanjeffers
dylanjeffers approved these changes May 18, 2023
View reviewed changes

@dylanjeffers dylanjeffers left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wonderful additions

@amendelsohn amendelsohn merged commit d38238d into main May 18, 2023
@amendelsohn amendelsohn deleted the audius-query-pagination branch May 18, 2023 18:01
audius-infra pushed a commit that referenced this pull request May 20, 2023
v1.5.25
23c7e34 
[9855fd2] Update locks (#3397) Dylan Jeffers
[aecef5f] [PAY-1144] [PAY-1182] [PAY-1147] DMs: Delete chat, message permissions (#3390) Marcus Pasell
[64ccd9d] Add stylelint to ci (#3373) Dylan Jeffers
[b642a16] [C-2518, C-2523, C-2611] Improve playlist create sagas (#3378) Dylan Jeffers
[5620d53] [PAY-1197] Mobile inbox unavailable modal from profile screen (#3376) Reed
[2ebaef5] [PAY-1248] Initial changes to get ready for upcoming PRs that include track and playlist tiles in DMs (#3391) Saliou Diallo
[c2fabf6] [PAY-1218] Mobile block dms drawer switches block/unblock (#3387) Reed
[0acad29] [C-2575] Match length of related artists user list to preview (#3395) Andrew Mendelsohn
[40525ae] [C-2615] Fix favorite tracks error due to empty track entries on web mobile (#3386) Kyle Shanks
[93a8077] [PAY-1145] DMs: Add InboxUnavailableModal (#3369) Marcus Pasell
[633484d] Update edit playlist flow and components (#3361) Kyle Shanks
[334ea0d] Special case ios safari for stem download (#3385) Raymond Jacobson
[d38238d] [C-2607] Pagination wrapper hooks for audius-query (#3375) Andrew Mendelsohn
[0a332d8] Hotfix: Fix stems downloads on mobile web (#3382) Marcus Pasell
[81da65e] Clean up artist_pick_track_id in APIUser (#3381) Michelle Brier
[64d89ef] Fix lint (#3380) Raymond Jacobson
[59129b4] [C-2614] Fix download stems mobile web (#3379) Raymond Jacobson
[df68727] [PAY-1032][PAY-892] Mobile DMs unread indicator, prefetch chats (#3352) Reed
[4a00fad] [PAY-1151] Handle chat reactions near top of screen on mobile (#3370) Reed
[bff316f] [PAY-1139] Throttle calls to fetchMessages on web scroll (#3372) Michael Piazza
[e723cec] C-2483 Fix queue overshot empty track player bug (#3353) nicoback2
[472a41d] [C-2596] Add disabled option to audius-query hooks (#3367) Andrew Mendelsohn
[f518962] [C-2602] Improve playlist library layout (#3364) Dylan Jeffers
[b3db8fa] Fix debounce on notif reaction (#3362) Raymond Jacobson
[2104b2b] [PAY-1183] Make clicking ChatUser handle/displayname lead to profile (#3368) Michael Piazza
[21cb6fc] DMs: Fix click handler in search user list for users you can't chat (#3358) Marcus Pasell
[24e7d8a] DMs: Update copy, scroll inbox-settings modal (#3359) Marcus Pasell
[e5e8d90] [PAY-941] Fix "1 new messages" unread tag (web) (#3366) Michael Piazza
[cfa1b2a] [C-2603] Fix readonly object error in audius-query reducer (#3365) Andrew Mendelsohn
[978c993] Fix invite reward claimable state on mobile (#3363) Reed
[a174fcc] [C-2556, C-2557] Address AI Attribution QA (#3349) Dylan Jeffers
[3c9b0f1] [PAY-1202] Refactor saved collections fetching (#3337) Randy Schott
[a0bdad5] Get call to action for chat permissions (#3325) Marcus Pasell
[03a2721] DMs: Use the optimistic unread count if applicable (#3354) Marcus Pasell
[8b99c2b] [C-2550] Left-nav fixes and improvements (#3357) Dylan Jeffers
[e7b0aab] [PAY-1215] Fix create new message crash (#3356) Reed
[6e7ece9] [PAY-1196] Mobile dms search users empty state (#3355) Reed
[315ae4f] [PAY-1219] Fix mobile chat reactions popup message getting cut off (#3342) Reed
@AudiusProject AudiusProject deleted a comment from linear Bot Sep 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@dylanjeffers dylanjeffers dylanjeffers approved these changes

@sliptype sliptype sliptype approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size/L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@amendelsohn @audius-infra @dylanjeffers @sliptype