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

Typescript error rest client custom method #2775

Closed
AshotN opened this issue Oct 4, 2022 · 13 comments · Fixed by #2788
Closed

Typescript error rest client custom method #2775

AshotN opened this issue Oct 4, 2022 · 13 comments · Fixed by #2788

Comments

@AshotN
Copy link
Contributor

AshotN commented Oct 4, 2022

Steps to reproduce

const restClient = rest("http://localhost:3030").fetch(
  window.fetch.bind(window)
);
const client = createClient(restClient);

client.use("user", restClient.service("user"), {
  methods: ["get", "create", "patch", "aCustomMethod"],
});

image

Actual behavior

TS2345: Argument of type 'Partial<ServiceMethods<any, Partial<any>, Params<Query>>>' is not assignable to parameter of type 'ClientService<{ first_name?: string | undefined; last_name?: string | undefined; role?: string | undefined; email?: string | undefined; password?: string | undefined; id: string; }, { id?: string | undefined; ... 4 more ...; password: string; }, { ...; }, Paginated<...>, Params<...>> & { ...; }'.   Type 'Partial<ServiceMethods<any, Partial<any>, Params<Query>>>' is not assignable to type 'ClientService<{ first_name?: string | undefined; last_name?: string | undefined; role?: string | undefined; email?: string | undefined; password?: string | undefined; id: string; }, { id?: string | undefined; ... 4 more ...; password: string; }, { ...; }, Paginated<...>, Params<...>>'.     Types of property 'find' are incompatible.       Type '((params?: Params<Query> | undefined) => Promise<any>) | undefined' is not assignable to type '(params?: Params<{ id?: string | { $gt?: string | undefined; $gte?: string | undefined; $lt?: string | undefined; $lte?: string | undefined; $ne?: string | undefined; $in?: string[] | undefined; $nin?: string[] | undefined; } | undefined; ... 7 more ...; $select?: DeepWritable<...>[] | undefined; }> | undefined) => ...'.         Type 'undefined' is not assignable to type '(params?: Params<{ id?: string | { $gt?: string | undefined; $gte?: string | undefined; $lt?: string | undefined; $lte?: string | undefined; $ne?: string | undefined; $in?: string[] | undefined; $nin?: string[] | undefined; } | undefined; ... 7 more ...; $select?: DeepWritable<...>[] | undefined; }> | undefined) => ...'.

System configuration

Tell us about the applicable parts of your setup.

Module versions :

"@feathersjs/authentication-client": "^5.0.0-pre.29",
"@feathersjs/feathers": "^5.0.0-pre.29",
"@feathersjs/rest-client": "^5.0.0-pre.29",
"typescript": "^4.8.4"

Everything else works, besides the Typescript error

@daffl
Copy link
Member

daffl commented Oct 4, 2022

The service types need to be expanded as noted in https://dove.feathersjs.com/api/client/rest.html#custom-methods with

import { CustomMethod } from '@feathersjs/feathers'

type ServiceTypes = {
  myservice: ClientService<...> & {
    myCustomMethod: CustomMethod<CustomMethodData, CustomMethodResponse>
  }
}

@AshotN
Copy link
Contributor Author

AshotN commented Oct 5, 2022

I do have that configured. Apologies for not including that in the original post, here is the relevant code

 user: ClientService<UserResult, UserData, UserPatch, Paginated<UserResult>, Params<UserQuery>> & {
    aCustomMethod: CustomMethod<any, any>
  }

@daffl
Copy link
Member

daffl commented Oct 6, 2022

What does the full client definition look like? I tested it out with an app generated by #2772 (which will be available shortly as pre.30) and I think it should be working (at least now).

@AshotN
Copy link
Contributor Author

AshotN commented Oct 7, 2022

pre.30 didn't fix it it caused a lot of other TS errors. For example on compile

$ shx rm -rf lib/ && tsc
node_modules/@feathersjs/schema/lib/json-schema.d.ts:2:25 - error TS2307: Cannot find module '@sinclair/typebox' or its corresponding type declarations.

import { TObject } from '@sinclair/typebox';
                          ~~~~~~~~~~~~~~~~~~~


Found 1 error in node_modules/@feathersjs/schema/lib/json-schema.d.ts:2

Had to add @sinclair/typebox to fix that error, still working through the rest*

@AshotN
Copy link
Contributor Author

AshotN commented Oct 7, 2022

I'm not sure if I misunderstood the docs or if it's a mistake

But in the example it says client.use('myservice', restClient.service('myservice'), {

Which I had implemented like on my client

import rest from "@feathersjs/rest-client";
import authentication from "@feathersjs/authentication-client";
import { createClient } from "aq-server";

const restClient = rest("http://localhost:3030").fetch(
  window.fetch.bind(window)
);
const client = createClient(restClient);

client.use("user", restClient.service("user"), {
  methods: ["get", "create", "patch", "aCustomMethod"],
});

client.configure(authentication());
export { client, restClient };

This caused the TS error, changing line 10 from restClient.service to client.service seems to fix it

client.use("user", client.service("user"), {
  methods: ["get", "create", "patch", "aCustomMethod"],
});

@daffl
Copy link
Member

daffl commented Oct 8, 2022

Your question has prompted me to add proper client side tests to a generated app in #2788. The setup that is now working looks like this in src/client.ts:

import { feathers } from '@feathersjs/feathers'
import type { Custom, CustomData, CustomQuery } from './services/customized/customized'

export type { Custom, CustomData, CustomQuery }
import type { Message, MessageData, MessageQuery } from './services/messages/messages'

export type { Message, MessageData, MessageQuery }
import type { Testing, TestingData, TestingQuery } from './services/path/to/test/test'

export type { Testing, TestingData, TestingQuery }
import type { User, UserData, UserQuery } from './services/users/users'

export type { User, UserData, UserQuery }
import type { Paginated, ClientService, TransportConnection, Params } from '@feathersjs/feathers'

export interface ServiceTypes {
  customized: ClientService<
    Custom,
    CustomData,
    Partial<CustomData>,
    Paginated<Custom>,
    Params<CustomQuery>
  > & {
    // Add custom methods here
  }
  messages: ClientService<
    Message,
    MessageData,
    Partial<MessageData>,
    Paginated<Message>,
    Params<MessageQuery>
  > & {
    // Add custom methods here
    customMethod(data: any, params?: Params): Promise<any>
  }
  'path/to/test': ClientService<
    Testing,
    TestingData,
    Partial<TestingData>,
    Paginated<Testing>,
    Params<TestingQuery>
  > & {
    // Add custom methods here
  }
  users: ClientService<User, UserData, Partial<UserData>, Paginated<User>, Params<UserQuery>> & {
    // Add custom methods here
  }
  //
}

export const createClient = <Configuration = any>(connection: TransportConnection<ServiceTypes>) => {
  const client = feathers<ServiceTypes, Configuration>()

  client.configure(connection)

  client.use('users', connection.service('users'), {
    // List all standard and custom methods
    methods: ['find', 'get', 'create', 'update', 'patch', 'remove']
  })

  client.use('path/to/test', connection.service('path/to/test'), {
    // List all standard and custom methods
    methods: ['find', 'get', 'create', 'update', 'patch', 'remove']
  })

  client.use('messages', connection.service('messages'), {
    // List all standard and custom methods
    methods: ['find', 'get', 'create', 'update', 'patch', 'remove', 'customMethod']
  })

  client.use('customized', connection.service('customized'), {
    // List all standard and custom methods
    methods: ['find', 'get', 'create', 'update', 'patch', 'remove']
  })

  return client
}

And is used in test/client.test.ts like this:

import assert from 'assert'
import axios from 'axios'

import rest from '@feathersjs/rest-client'
import authenticationClient from '@feathersjs/authentication-client'
import { app } from '../src/app'
import { createClient } from '../src/client'
import type { UserData } from '../src/client'

const port = app.get('port')
const appUrl = `http://${app.get('host')}:${port}`

describe('application client tests', () => {
  const client = createClient(rest(appUrl).axios(axios))
  client.configure(authenticationClient())

  before(async () => {
    await app.listen(port)
  })

  after(async () => {
    await app.teardown()
  })

  it('initialized the client', () => {
    assert.ok(client)
  })

  it('creates and authenticates a user with email and password', async () => {
    const userData: UserData = {
      email: 'someone@example.com',
      password: 'supersecret'
    }

    await client.service('users').create(userData)

    const { user, accessToken } = await client.authenticate({
      strategy: 'local',
      ...userData
    })

    assert.ok(accessToken, 'Created access token for user')
    assert.ok(user, 'Includes user in authentication data')
    assert.strictEqual(user.password, undefined, 'Password is hidden to clients')

    const response = await client.service('messages').customMethod('something')

    assert.ok(response)

    await client.logout()

    // Remove the test user on the server
    await app.service('users').remove(user.id)
  })
})

@daffl
Copy link
Member

daffl commented Oct 8, 2022

I'm actually wondering if for custom methods to keep them in sync with the actual signatures we should do something like

import type { MessageService } from './services/messages/messages'

export interface ServiceTypes {
  messages: ClientService<
    Message,
    MessageData,
    Partial<MessageData>,
    Paginated<Message>,
    Params<MessageQuery>
  > & Pick<MessageService, 'customMethod'>
}

@daffl
Copy link
Member

daffl commented Oct 8, 2022

In fact, this totally works the way it should:

import { defaultServiceMethods, feathers } from '@feathersjs/feathers'
import type { TransportConnection } from '@feathersjs/feathers'

import type { Message, MessageData, MessageQuery, MessageService } from './services/messages/messages'
export type { Message, MessageData, MessageQuery }

const defaultServiceMethods = ['find', 'get', 'create', 'update', 'patch', 'remove'] as const

const messageServiceMethods = [...defaultServiceMethods, 'customMethod'] as const
type MessageServiceMethods = typeof messageServiceMethods[number]

export interface ServiceTypes {
  messages: Pick<MessageService, MessageServiceMethods>
}

export const createClient = <Configuration = any>(connection: TransportConnection<ServiceTypes>) => {
  const client = feathers<ServiceTypes, Configuration>()

  client.configure(connection)

  client.use('messages', connection.service('messages'), {
    // List all standard and custom methods
    methods: messageServiceMethods as any
  })

  return client
}

So we basically get all the actual valid, always up to date service methods (except that you could do { paginate: false } on the client which wouldn't work without additional - but possible - hooks).

@AshotN
Copy link
Contributor Author

AshotN commented Oct 8, 2022

One idea I had which I'm not sure if is entirely possible or feasible. I really dislike using strings to represent the custom functions. Like [...defaultServiceMethods, 'customMethod'] It makes refactoring in the future a little harder.

Would it be possible for us to avoid strings?

@daffl
Copy link
Member

daffl commented Oct 8, 2022

Unfortunately not. The reason is that we need an actual array for the methods to initialise them dynamically on the client services. This is not possible with only types (the methods array is used in e.g. https://github.com/feathersjs/feathers/blob/dove/packages/rest-client/src/index.ts#L59).

I still think it's nicer than the previous ClientService interface where you have to define everything manually and get no help with refactoring at all - it would just break at runtime if you e.g. declare a custom method with the wrong types or that doesn't exist on the server. Where in the above case you'd get a compilation error if e.g. a method is listed that doesn't exist on the service.

@AshotN
Copy link
Contributor Author

AshotN commented Oct 8, 2022

Understandable, once your PR is merged I will regenerate my services and test everything. Thanks for your help

@daffl
Copy link
Member

daffl commented Oct 8, 2022

I was wondering if we could re-use the methods array from the server side service registration but I'm concerned that - unlike import type which will always get removed at compile time - we might accidentally end up with server code in the client build. Let's try it with the above approach for now and see if there is a way to iterate.

Thank you for your help in testing this out!

@AshotN
Copy link
Contributor Author

AshotN commented Oct 8, 2022

Would something like this be reasonable, or is to confusing?

const ourFunc = () => {
}

console.log(ourFunc.name) //ourFunc 

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