feathers

feathers copied to clipboard

Steps to reproduce

In a fresh copy of feathers-chat, paste the following snippet at the end of the app.ts file:

(async () => {
  const query: any = {
    text: 'foo'
  }
  const messages = await app.service('messages').find({
    query,
    paginate: false
  })
  messages.forEach(() => {})
})

Expected behavior

There should be no Typescript errors.

Actual behavior

Property 'forEach' does not exist on type 'Paginated<{ id: number; user: { password?: string | undefined; githubId?: number | undefined; avatar?: string | undefined; id: number; email: string; }; text: string; createdAt: number; userId: number; }>'.

Workaround

Using explicit type assertion on query props resolves the errors:

(async () => {
  const query: any = {
    text: 'foo'
  }
  const messages = await app.service('messages').find({
    query: {
      text: query.text as string
    },
    paginate: false
  })
  messages.forEach(() => {})
})

Possibly related to https://github.com/feathersjs/feathers/issues/3012 as I think both issues suffer from the way HookContext interprets the current Service<Params>.

Discord thread

System configuration

Module versions: v5

NodeJS version: 16,18

Operating System: linux

Browser Version: -

React Native Version: -

Module Loader: es6

So I can definitely confirm this is a problem and I found a fix for this use case (by removing https://github.com/feathersjs/feathers/blob/dove/packages/mongodb/src/index.ts#L16) but then the paginated case doesn't work.

I also haven't found any TypeScript references why it works with the workaround but not with query as a variable.

Hey, I think I found the problem,

ServiceParams extends AdapterParams, which looks like { adapter?: A; paginate?: PaginationParams; }

Where

type PaginationParams = false | PaginationOptions;

Therefore using { paginate: false } will use the first overload of "find" rather than the literal "false" in the second overload

The problem could easily be fixed short term by reversing the order of the overloads in the mongodb and knex packages

Further to this, I've found that the returned result (not just the type) is always a paginated object, despite specifying paginate: false.

Upon further digging, it seems that the pagination value set in the app config is always taking preference over that of the method call.

This issue is blocking us from upgrading Typescript from 4 to 5 in our project currently. There is something really strange going on with the function overload matching. I tried simplifying the overload pattern as much as possible while still replicating the issue, the example below also gives the wrong return type for result, was expecting number but get string.

type SomeNumbers = { some?: number; number?: number }

type test = {
  someOtherProp?: string
  paginate?: false | SomeNumbers
}

function testing(params?: test & { paginate?: SomeNumbers }): string
function testing(params?: test & { paginate: false }): number
function testing(params?: test): string | number {
  return params?.paginate === false ? 123 : "123"
}

const input = { paginate: false }
const result = testing(input as { paginate: false }) // result: string 😕

Simple change like removing someOtherProp from test type or setting some to non optional within SomeNumbers will result in the correct overload returning number being selected instead, so not sure what is going on with Typescript here.

As @kieran-mgc have already pointed out, moving the overload with paginate: false up to the top in index.ts seems to solve the issue, so that could be an possible workaround for feathers

async find(params?: ServiceParams & { paginate: false }): Promise<Result[]>
async find(params?: ServiceParams & { paginate?: PaginationOptions }): Promise<Paginated<Result>>
async find(params?: ServiceParams): Promise<Paginated<Result> | Result[]>
async find(params?: ServiceParams): Promise<Paginated<Result> | Result[]> {

We solved it in our project by using this class instead, with different order of function overloads for find

export class KnexServiceFixed<
  Result = any,
  Data = Partial<Result>,
  ServiceParams extends Params<any> = KnexAdapterParams,
  PatchData = Partial<Data>,
> extends KnexService<Result, Data, ServiceParams, PatchData> {
  async find(params?: ServiceParams & { paginate: false }): Promise<Result[]>
  async find(
    params?: ServiceParams & { paginate?: PaginationOptions },
  ): Promise<Paginated<Result>>
  async find(params?: ServiceParams): Promise<Paginated<Result> | Result[]>
  async find(params?: ServiceParams): Promise<Paginated<Result> | Result[]> {
    return super.find(params)
  }

  async _find(params?: ServiceParams & { paginate: false }): Promise<Result[]>
  async _find(
    params?: ServiceParams & { paginate?: PaginationOptions },
  ): Promise<Paginated<Result>>
  async _find(params?: ServiceParams): Promise<Paginated<Result> | Result[]>
  async _find(
    params: ServiceParams = {} as ServiceParams,
  ): Promise<Paginated<Result> | Result[]> {
    return super._find(params)
  }
}

Still running into this precise issue. Any plans to get this rolled into Feathers 5 latest?