Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix got.paginate(...) typings #1099

Merged
merged 6 commits into from Mar 24, 2020
Merged

Fix got.paginate(...) typings #1099

Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bc51259
fix: typings for paginate
jaulz Mar 3, 2020
665b5e3
style: fix lint
Mar 3, 2020
7c8b888
fix: use Omit instead of Except
jaulz Mar 20, 2020
afdb1fa
fix: revert use of Omit
jaulz Mar 20, 2020
db07241
fix: add URLOrGotPaginateOptions type
Mar 24, 2020
c8169b9
test: add types to tests
Mar 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 10 additions & 8 deletions source/create.ts
View file
Open in desktop
@@ -1,4 +1,4 @@
import {Merge} from 'type-fest';
import {Merge, Except} from 'type-fest';
import is from '@sindresorhus/is';
import asPromise, {createRejection} from './as-promise';
import asStream, {ProxyStream} from './as-stream';
Expand Down Expand Up @@ -61,9 +61,11 @@ export interface GotRequestMethod {
<T>(url: string | Merge<Options, {isStream: true}>, options?: Merge<Options, {isStream: true}>): ProxyStream<T>;
}

export type GotPaginateOptions<T> = Except<Options, keyof PaginationOptions<unknown>> & PaginationOptions<T>;

export interface GotPaginate {
<T>(url: URLOrOptions & PaginationOptions<T>, options?: Options & PaginationOptions<T>): AsyncIterableIterator<T>;
all<T>(url: URLOrOptions & PaginationOptions<T>, options?: Options & PaginationOptions<T>): Promise<T[]>;
<T>(url: string | GotPaginateOptions<T>, options?: GotPaginateOptions<T>): AsyncIterableIterator<T>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think string | GotPaginateOptions<T> should be URLOrOptions & GotPaginateOptions<T>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would duplicate work but also duplicate the options because GotPaginateOptions is defined like this:

export type GotPaginateOptions<T> = Except<Options, keyof PaginationOptions<unknown>> & PaginationOptions<T>;

Though, I just pushed a commit which adds the URLOrGotPaginateOptions type so we have that definition in one place.

all<T>(url: string | GotPaginateOptions<T>, options?: GotPaginateOptions<T>): Promise<T[]>;
}

export interface Got extends Record<HTTPAlias, GotRequestMethod>, GotRequestMethod {
Expand Down Expand Up @@ -198,8 +200,8 @@ const create = (defaults: Defaults): Got => {
}

// @ts-ignore The missing property is added below
got.paginate = async function * <T>(url: URLOrOptions, options?: Options) {
let normalizedOptions = normalizeArguments(url, options, defaults);
got.paginate = async function * <T>(url: string | GotPaginateOptions<T>, options?: GotPaginateOptions<T>) {
let normalizedOptions = normalizeArguments(url as URLOrOptions, options as Options, defaults);

const pagination = normalizedOptions._pagination!;

Expand Down Expand Up @@ -245,11 +247,11 @@ const create = (defaults: Defaults): Got => {
}
};

got.paginate.all = async <T>(url: URLOrOptions, options?: Options) => {
got.paginate.all = async <T>(url: string | GotPaginateOptions<T>, options?: GotPaginateOptions<T>) => {
const results: T[] = [];

for await (const item of got.paginate<unknown>(url, options)) {
results.push(item as T);
for await (const item of got.paginate<T>(url, options)) {
results.push(item);
}

return results;
Expand Down