node-zendesk icon indicating copy to clipboard operation
node-zendesk copied to clipboard

Types do not match code

Open stormsa opened this issue 3 years ago • 4 comments

Hello,

I found your lib really useful and thank you for it. I use it with typescript and I try to use it with @types/node-zendesk : https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/node-zendesk

The library was supposed to define your code but it does not :

Example with show user : interface ResponsePayload { user: ResponseModel; } show(userId: ZendeskID): Promise<ResponsePayload>;

But your code returns directly ReponseModel.

Zendesk also returns user in a parent object.

So did you decide to return children parent directly or I made a mistake somewhere ?

Thank you

stormsa avatar Feb 21 '22 17:02 stormsa

node-zendesk version : 2.0.0 or 2.1.0 @types/node-zendesk 2.0.6 zendesk documentation : https://developer.zendesk.com/api-reference/ticketing/users/users/#show-user

stormsa avatar Feb 21 '22 17:02 stormsa

Also running into this issue!

SamSokolin avatar Mar 22 '22 18:03 SamSokolin

same issue on creating a ticket

brendanmorrell avatar Jul 17 '22 01:07 brendanmorrell

Running into this issue as well. If I modify the file: node_modules/node-zendesk/lib/client/client.js by commenting out / deleting lines 316-326 it works as expected with the types definition. (at least for the tickets api).

I'd prefer that the objects returned from the calls match the zendesk documentation (i.e. returning the parent object, rather than the child object).

Here is the commented out code that is causing the issue for reference of others dealing with this issue: // if (self.jsonAPINames) { // self.jsonAPINames.push("audit"); // self.jsonAPINames.push("via"); // for (var i = 0; i < self.jsonAPINames.length; i++) { // if (res.hasOwnProperty(self.jsonAPINames[i].toString())) { // body = res[self.jsonAPINames[i].toString()]; // break; // } // } // }

jase-k avatar Aug 29 '22 13:08 jase-k

I'm facing the same issue.

I'm planning to use a similar solution to what @jase-k suggested, though wrap the node-zendesk client creation with my own function that will remove the jsonAPINames property from each of the APIs.

const createZendesk = (options: ClientOptions): zendesk.Client => {
  const client = zendesk.createClient(options)

  Object.keys(client).forEach((key) => {
    if (typeof client[key] === 'object' &&
      Object.prototype.hasOwnProperty.call(client[key], 'jsonAPINames')
    ) {
      delete client[key].jsonAPINames
    }
  })

  return client
}

mralbean avatar Jun 28 '23 22:06 mralbean

The issue still seems to be there, unfortunately.

Seems like a few returns have wrong types. .users.search and .requests.create for example.

maciekpaprocki avatar Aug 30 '23 17:08 maciekpaprocki

@maciekpaprocki, thanks for drawing attention to this. 😊 I'm reopening the issue.

Historically, the types for this project have been managed through DefinitelyTyped contributions, particularly here. Admittedly, after spending an hour diving into it, some nuances still elude me.

I'm genuinely committed to supporting Types. Over the past few weeks, there's been a major refactor: removing certain dependencies, replacing request with the native fetch, and generally modernizing the library. 🚀 My next big task, albeit a daunting one, is to extensively comment on all client methods. I anticipate this will serve a dual purpose: enhancing the documentation and facilitating type generation. If I'm not mistaken, a command like

npx -p typescript tsc src/**/*.js --declaration --allowJs --emitDeclarationOnly --outDir types

should, in theory, read my JSDoc annotations and generate the types accordingly.

But, that's about as far as I've planned. 🤔 Post auto-generation of the types, I'm a bit at sea regarding the next steps. If you or anyone else has insights on this, I'd truly appreciate a step-by-step rundown.

Just to clarify: I hear you loud and clear. I want the community to know I'm actively working on it. 💪

blakmatrix avatar Aug 31 '23 06:08 blakmatrix

Dude, you are great. It's really not a big deal. It's an extra addon to have types, but without it, it's very good library either way ( definitely better than zendesk doc insanity )

I am not gonna lie, I am not a big fan of ts and js in general so I am avoiding insanity of types in both as much as I can. Just noticed an issue on project I am working on kind of against my will :)

Said that, if you are developing new version, maybe creating it in typescript would be more useful. I am not sure where you are ( and if further down it might be insane project to undertake ), but typescript types are easier to write than jsdoc as they are checked automatically ( don't know if there's any validation tool for types )

maciekpaprocki avatar Sep 04 '23 21:09 maciekpaprocki

Same here waiting for types to be aligned.

For now I have a workaround to cast the returned values as unknown and then as the desired types:

  getTicket = async (id: number): Promise<Tickets.ResponseModel> => {
      return this.client.tickets.show(id) as unknown as Tickets.ResponseModel;
  };

This way, the flows which are using my own implementation which wrapps the zendesk client calls, can infer and use object fields of the zendesk entities.

ofir5300 avatar Sep 05 '23 06:09 ofir5300

🚀 Release Announcement: Node-Zendesk v4.0.0 🚀

I'm thrilled to announce the release of v4.0.0. This release marks a significant refactor that I believe will provide stability for future iterations. Moving forward, I envision that any changes will be additive or in the form of deprecations.

I've taken a glance at the TypeScript definitions on DefinitelyTyped and, to be honest, I'm not entirely certain about its inner workings. I understand that the recent changes might necessitate a considerable update in the type definitions. If someone is up for this task, I can assure you it will be a valuable contribution, considering the robust foundation laid down by v4.0.0.

Furthermore, I'm currently sifting through the Zendesk API endpoints to identify changes, sunset features, and new additions. As I navigate through these, I'm actively enriching the project with JSDoc comments. An updated documentation is also in the pipeline and will be released soon.

Looking forward to the community's support and collaboration!

blakmatrix avatar Sep 13 '23 05:09 blakmatrix

moving the types to the repository here instead of https://github.com/DefinitelyTyped/DefinitelyTyped would be a smart move and would allow quicker changes to the types as Zendesk types changes.

considering the current repository is all written in js moving to typescript would be a great effort instead using jsdocs will be quicker to start typing the methods and generating .d.ts files.

@blakmatrix if you would like I can start a branch and start the integration with jsdocs, what do you tell me?

onhate avatar Oct 03 '23 18:10 onhate

It's enough of a pain point for folks that I think it could be a great Idea

blakmatrix avatar Oct 03 '23 20:10 blakmatrix

@ofir5300 @stormsa @SamSokolin @brendanmorrell @maciekpaprocki @mralbean @jase-k this should be solved now :) checkout https://github.com/blakmatrix/node-zendesk/releases/tag/v5.0.0

blakmatrix avatar Oct 10 '23 08:10 blakmatrix

I don't think this resolves all the type necessities, at least on the the return type side of things, it is lacking type properties (return is mostly typed as object / object[] / any). Or am I missing a certain step in using the new library based typings?

Screenshot 2023-10-11 at 14 22 40

stasas avatar Oct 11 '23 12:10 stasas

@stasas we have just setup the repository to generate the types directly from here without depending on the Definitely-typed thing that constantly get outdated. Being able to generate types from the lib code itself will guarantee agility on updating the lib for the changes, but the downside of this is that we need now to bring all the types to the lib. We are eager to find help on this enormous task, if you can help typing the tickets api please open a PR and help us make this lib fully and correctly typed.

onhate avatar Oct 11 '23 12:10 onhate