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.

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

[Types] Make queryParams strongly-typed #101

Closed
sewera opened this issue Dec 6, 2022 · 5 comments
Closed

[Types] Make queryParams strongly-typed #101

sewera opened this issue Dec 6, 2022 · 5 comments

Comments

@sewera
Copy link

sewera commented Dec 6, 2022

Right now, there are no types for queryParams; the default value is {}, so any object will match this type.

Some autocomplete will be nice, so I propose such alternative:

type GetListQueryParams = {
  page?: number
  perPage?: number
  sort?: string
  filter?: string
  expand?: string
}

// BaseCrudService@41
protected _getList<T = M>(basePath: string, page = 1, perPage = 30, queryParams: GetListQueryParams = {}): Promise<ListResult<T>> {
// ...
}
@sewera
Copy link
Author

sewera commented Dec 6, 2022

If it's okay with you @ganigeorgiev, I can make those changes and open a PR.

I can also merge the BaseCrudService and CrudService (resolve this todo) if you want me to :)

@ganigeorgiev
Copy link
Member

ganigeorgiev commented Dec 6, 2022

Hm, this was requested previously as part of #64.

For now let's keep the scope of the issue minimal and not merge the BaseCrudService and CrudService since this will require some major updates to the tests too.

We can add type for the query params but the declaration(s) needs to be a little more "relaxed" for better backward comparability and to allow setting also custom query props, eg. something like:

export interface BaseQueryParams {
    [key: string]: any;

    expand?:      string;
    $autoCancel?: boolean;
    $cancelKey?:  string;
}

export interface ListQueryParams extends BaseQueryParams {
    page?:    number;
    perPage?: number;
    sort?:    string;
    filter?:  string;
}

@sewera If you don't have the time to work on this, I'll implement it sometime at the end of the week.

@sewera
Copy link
Author

sewera commented Dec 6, 2022

I'll try to put together a PR with those changes by tomorrow.

sewera added a commit to sewera/pocketbase-js-sdk that referenced this issue Dec 7, 2022
- make interfaces small and composable
- fix typos in log messages and comments
sewera added a commit to sewera/pocketbase-js-sdk that referenced this issue Dec 7, 2022
@sewera
Copy link
Author

sewera commented Dec 7, 2022

@ganigeorgiev, please find PR #102 ready for review.

@ganigeorgiev
Copy link
Member

The following types were added in master:

  BaseQueryParams
  ListQueryParams
  RecordQueryParams
  RecordListQueryParams
  LogStatsQueryParams
  FileQueryParams

They will be available with the v0.8.4 release sometime later this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants