postman-collection icon indicating copy to clipboard operation
postman-collection copied to clipboard

TypeScript type definitions

Open kbuzby opened this issue 6 years ago • 17 comments

Hello! I've written TypeScript type definitions and am submitting them to DefinitelyTyped: DefinitelyTyped/DefinitelyTyped#24227

I just wanted to notify here in case you would like to review and there is anything that should be updated in them, or if you would like to try and bundle the types with your npm package (since that is the preferred method) then I would be happy to help coordinate a pull request or anything else you need to make that happen.

kbuzby avatar Mar 12 '18 02:03 kbuzby

This is amazing. I will dig deep. @kamalaknn and @harryi3t will also look into it.

shamasis avatar May 14 '18 20:05 shamasis

@kbuzby - is there a way you suggest where we can generate these declarations from jsdoc? We use jsdoc to generate documentation (that has type in them.) We could use something like ted-jsdoc to generate the types.

  • https://www.npmjs.com/package/tsd-jsdoc
  • https://github.com/brettle/jsdoc-to-typescript-declaration

We could put in the time to coordinate and have this in our repo and would love to have you onboard as a contributor.

PS: We could also use the schema for this (maybe... not sure) https://schema.getpostman.com/json/collection/v2.1.0/docs/index.html

shamasis avatar Jun 26 '18 08:06 shamasis

@shamasis I'd be more than happy to work on this - the first time I didn't really think about generating the type definitions from the documentation because I had just used the built in DefinitelyTyped type generator (dts-gen) which looks at the code to generate a starting point.

There might still be some need for manual review of the output (at least at first until the docs 100% align) to make sure the types generated match how it works and how it's meant to be used.

I'll look into the two libraries you linked and see if there's anything else as well over the weekend.

Not sure about using the schema as well - it would give property information about the classes, but nothing about the method signatures (both instance and static). Maybe it could still be used in the verification process.

kbuzby avatar Jun 29 '18 19:06 kbuzby

Has anything come of this?

myahl avatar Jul 30 '20 16:07 myahl

@kbuzby @shamasis Please, don't use types from DefinitelyTyped for now. Why? Because it's not the same types as they currently are in the postman-collection library. I used Collection from this library and when I switched to @types/postman-collection the constructor of the Collection class broke. I passed the JSON object to parse into Collection type

rafek1241 avatar Aug 25 '20 18:08 rafek1241

Ah. We have been doing some work where we needed to do the jsDoc and types better ... that's for the "autocomplete" feature in Postman's script editors.

Perhaps those can get formally into the repo and actually add some value here?!

@codenirvana thoughts?

shamasis avatar Aug 25 '20 18:08 shamasis

Hey @shamasis,

I can see that the types are now bundled in the repository, but the definitions apparently seem to be outdated/incorrect. How come did you depart from the definitely typed ones? Usually speaking, it's advised not to bundle your typings with the repository itself unless the repository itself is a TypeScript one. What is the plan for this?

XVincentX avatar Sep 28 '20 18:09 XVincentX

@XVincentX the definitely typed was a community contribution and the bundled types are auto-generated using the existing JSDoc. We didn't want the overhead of updating two sources to it's best to auto-generate before the release process.

I agree that most of the non-TS packages don't bundle types but honestly, I don't see any issue with this. Please let me know if I am missing something or you are facing any issue because of this change.

codenirvana avatar Sep 28 '20 18:09 codenirvana

@codenirvana Thanks for the prompt reply.

From what I can see, the types bundled are apparently missing some methods that appear to be still part of the library. For example:

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/postman-collection/index.d.ts#L60 https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/postman-collection/index.d.ts#L9

P.S: I wrote some of these definitions

XVincentX avatar Sep 28 '20 18:09 XVincentX

@XVincentX First of all, I really appreciate your hard work for maintaining the definitely typed definitions.

Now, toObjectResolved is missing because it's a private method and PropertyBaseDefinition is available here.

Also, We are very new to TS and this is our first attempt to generate the types using tsd-jsdoc, build script. Since you already have a good context about Postman Collection types, I would like you to please review the bundled types available here.

Moving forward we would like to keep types closer to the project source, like how fastify does. Additionally, this change really helped us improve our JSDoc definition and generated documentation 😅

codenirvana avatar Sep 28 '20 19:09 codenirvana

Thanks again for the reply.

I understand your reasons and intentions.

Unfortunately this is going too far in terms of the time I can dedicate into this (keeping the Postman types up to date was already not that easy), so I'm going to jump off from the discussion.

If these are going to be the official types, you should open a PR to the definitelyTyped repositories and mark the types there as deprecated, so that people do not go in confusion for that.

Now, toObjectResolved is missing because it's a private method

Well yes, but keep in mind there's no such private thing in JavaScript. You can hide it in TypeScript, but the method is still going to be there. I'd personally rather focus on completeness, but that's arguable.

and PropertyBaseDefinition is available here.

I can see some objects are now missing the description property, which was present and working previously without problems. https://github.com/postmanlabs/postman-collection/blob/develop/types/index.d.ts#L185 would be an example.

XVincentX avatar Sep 28 '20 19:09 XVincentX

It seems that RequestAuth.definition is missing AuthTypes types https://github.com/postmanlabs/postman-collection/blob/38e3d0ecdcd0eaefb86bcec3934d8f97ff87acb0/types/index.d.ts#L1648

b-pagis avatar Nov 15 '20 20:11 b-pagis

The bundled types broke the definitely typed package for newman, https://github.com/DefinitelyTyped/DefinitelyTyped/issues/48543

rohit-gohri avatar Dec 07 '20 19:12 rohit-gohri

Looks like bundled types are affecting a lot of your use cases.

Since we don't have enough bandwidth to fix the auto-generated types at the moment... we'll just remove the "types" from the package.json and make a release.

Hope that works!

codenirvana avatar Dec 07 '20 19:12 codenirvana

I think you'll have better luck with Typescript's allowJs and emit declarations: https://www.typescriptlang.org/docs/handbook/declaration-files/dts-from-js.html You can also start type checking js code that way with jsdoc. Though TS flavoured jsdoc and jsdoc aren't always compatible.

rohit-gohri avatar Dec 07 '20 19:12 rohit-gohri

As mentioned in https://github.com/postmanlabs/postman-collection/issues/602#issuecomment-740129552, released v3.6.9 without specifying the bundled declarations in the package.json (removed types property).

codenirvana avatar Jan 02 '21 17:01 codenirvana

so which types should i be using / trying to fix? i'm attempting to fix the type definitions for postman-collection in https://github.com/postmanlabs/postman-sandbox/pull/826 but neither the types in postman-collection or @types/postman-collection seem to be correct

DetachHead avatar May 25 '22 04:05 DetachHead