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

PaginateConfig has string default type for some attributes even it's a generic interface #866

Closed
sitoftonic opened this issue Feb 3, 2024 · 2 comments · Fixed by #931
Closed

Comments

@sitoftonic
Copy link

While using the PaginateConfig interface, when trying to provide values to filterableColumns or select attributes, I cannot get hints about real attributes in the entity, even that PaginateConfig has a required generic type.

For example, having this:

const config: PaginateConfig<MeetingEntity> = {
  filterableColumns: {
    name: true,
    // Cursor placed here
  },
};

And placed just below the name: true, when pressing CTRL+I in VSCode to get hints about other attributes in MeetingEntity, I cannot get the attributes. The definition of PaginateConfig is as follows:

export interface PaginateConfig<T> {
  relations?: FindOptionsRelations<T> | RelationColumn<T>[] | FindOptionsRelationByString
  sortableColumns: Column<T>[]
  nullSort?: 'first' | 'last'
  searchableColumns?: Column<T>[]
  select?: Column<T>[] | string[]
  maxLimit?: number
  defaultSortBy?: SortBy<T>
  defaultLimit?: number
  where?: FindOptionsWhere<T> | FindOptionsWhere<T>[]
  filterableColumns?: {
      [key in Column<T> | string]?: (FilterOperator | FilterSuffix)[] | true
  }
  loadEagerRelations?: boolean
  withDeleted?: boolean
  paginationType?: PaginationType
  relativePath?: boolean
  origin?: string
  ignoreSearchByInQueryParam?: boolean
  ignoreSelectInQueryParam?: boolean
}

https://github.com/ppetzold/nestjs-paginate/blob/master/src/paginate.ts#L68

To solve the problem, the | string in both attributes should be removed, as there is no valid way PaginateConfig does not have a type provided and defaulting to string breaks previous Column<T> typing. This also bypasses static analysis for non existing attributes.

I have tried to create a PR with these changes but I'm not allowed to push to the repository.

@Helveg
Copy link
Collaborator

Helveg commented Mar 3, 2024

Hi, to create a pull request, please fork this repository, and open a pull request across forks :)

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request-from-a-fork

@Helveg
Copy link
Collaborator

Helveg commented Jun 5, 2024

@ppetzold can I get your input here. I would guess that the | string is to allow people to bypass the hinting in case the TypeScript compiler can't infer all the valid values?

Perhaps this is stopping some IDEs from autocompleting at all? It is probably due to the problem described here: microsoft/TypeScript#29729 and we could try to use one of the fixes in the thread?

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