Add missing native node tls.checkServerIdentity function #21897
Conversation
|
types/node/index.d.ts to authors (@DefinitelyTyped/DefinitelyTyped @parambirs @tellnes @WilcoBakker @octo-sniffle @smac89 @Flarna @mwiktorczyk @wwwy3y3 @DeividasBakanas @kjin @alvis @OliverJAsh @eps1lon Microsoft TypeScript (account can't be detected)). Could you review this PR? |
| // TODO: change to `type NodeRequireFunction = (id: string) => any;` in next mayor version. | ||
| /* tslint:disable:callable-types */ | ||
| interface NodeRequireFunction { | ||
| /* tslint:disable-next-line:callable-types */ |
4e7d865 to
90dba6a
Compare
|
Added Did not remove the lint fix as the linked PR hasn't been merged |
90dba6a to
2f001c0
Compare
|
@Hannes-Magnusson-CK Please fix the failures indicated in the Travis CI log. |
|
Travis is passing. I had a syntax error for a second that was fixed before that comment was added |
types/node/index.d.ts
Outdated
| path?: string; | ||
| ALPNProtocols?: Array<string | Buffer>; | ||
| checkServerIdentity?: (servername: string, cert: string | Buffer | Array<string | Buffer>) => any; | ||
| checkServerIdentity?: CheckServerIdentityFunction; |
There was a problem hiding this comment.
Isn't it possible to write checkServerIdentity?: typeof checkServerIdentity here and avoid the extra type CheckServerIdentityFunction?
This would ensure that the function and the property here are always in sync.
There was a problem hiding this comment.
The only use of typeof in this file was in the Global interface definition, so it seemed like it wasn't expected syntax to use, so I followed the lead of LookupFunction.
If you think using typeof directly and removing the type declaration is kosher in this file, then I'll fix :D
There was a problem hiding this comment.
The reason why I used a dedicated type for LookupFunction instead typeof dns.lookup is that dns.lookup offers more options that the variant used by sockets and I don't think there is a typeof operand to select just the type of one specific overload.
But here we don't have this special case therefore I would suggest to go for typeof - except someone else (or CI) explains why not.
There was a problem hiding this comment.
Gotcha. Fixed :)
2f001c0 to
a385b18
Compare
|
Approved by a listed owner. PR ready to merge pending express review by a maintainer. |
npm run lint package-name(ortscif notslint.jsonis present).If changing an existing definition: