-
Notifications
You must be signed in to change notification settings - Fork 39
feat: Add asyncIterator to the StoreCollectionClient.list return value
#790
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
8c38f94
68ed7a3
de2c43f
d9f28d3
5e4cb99
9e108d9
bd4a534
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,9 @@ export class StoreCollectionClient extends ResourceCollectionClient { | |
| /** | ||
| * https://docs.apify.com/api/v2/#/reference/store/store-actors-collection/get-list-of-actors-in-store | ||
| */ | ||
| async list(options: StoreCollectionListOptions = {}): Promise<PaginatedList<ActorStoreList>> { | ||
| list( | ||
| options: StoreCollectionListOptions = {}, | ||
| ): Promise<PaginatedList<ActorStoreList>> & AsyncIterable<ActorStoreList> { | ||
| ow( | ||
| options, | ||
| ow.object.exactShape({ | ||
|
|
@@ -33,7 +35,7 @@ export class StoreCollectionClient extends ResourceCollectionClient { | |
| }), | ||
| ); | ||
|
|
||
| return this._list(options); | ||
| return this._getIterablePagination(options); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It can be easily applied to other similar endpoints, but maybe it is better to do it gradually to limit the size of the change? |
||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -233,6 +233,20 @@ export interface PaginationIteratorOptions { | |||||
| exclusiveStartId?: string; | ||||||
| } | ||||||
|
|
||||||
| export interface PaginationOptions { | ||||||
| /** Position of the first returned entry. */ | ||||||
| offset?: number; | ||||||
| /** Maximum number of entries requested. */ | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
not sure if
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is actually the total limit for the whole iterator. Chunk size is limited by the length of the platform response; it is not limited by this code. |
||||||
| limit?: number; | ||||||
| } | ||||||
|
|
||||||
| export interface PaginatedResponse<Data> { | ||||||
| /** Total count of entries. */ | ||||||
| total: number; | ||||||
| /** Entries. */ | ||||||
| items: Data[]; | ||||||
| } | ||||||
|
|
||||||
| export interface PaginatedList<Data> { | ||||||
| /** Total count of entries in the dataset. */ | ||||||
| total: number; | ||||||
|
|
@@ -248,6 +262,8 @@ export interface PaginatedList<Data> { | |||||
| items: Data[]; | ||||||
| } | ||||||
|
|
||||||
| export interface IterablePaginatedList<Data> extends PaginatedList<Data>, AsyncIterable<PaginatedList<Data>> {} | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think we could just enhance the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we need to keep both types, because internally we will keep using the old |
||||||
|
|
||||||
| export function cast<T>(input: unknown): T { | ||||||
| return input as T; | ||||||
| } | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if there is some type-scriptish way to do this without the
askeywordas unknown as AsyncIterable<Data> & Promise<R>;There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
afaik you can make a better-typed
definePropertymethod like this:This implementation will cast the return type to the original type
& { key: typeof value }.I'm not entirely sure if it's worth the extra 8 lines. It's a rather hacky solution, so explicit cast is IMO okay here.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To completely avoid any casts, I could do something like this. Is it worth it? Or should I just leave it as it is with
as unknown as AsyncIterable<Data> & Promise<R>;and use it in a simple way:
new IterablePromise<Data, R>(paginatedListPromise, asyncIterator);There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...And I was doubting whether adding 8 LOC would be too much 😅
IMO it's not necessary, especially if it can be solved by one cast (admittedly, not a good practice, but here it's afaiac fine).