search

typings / registry

The registry of type definitions for TypeScript
238 stars 176 forks source link

update to latest ldapjs type defs #988

Closed nickzelei closed 7 years ago

nickzelei commented 7 years ago

Typings URL: https://github.com/types/npm-ldapjs

Questions (for new typings):

Change Summary (for existing typings):

blakeembrey commented 7 years ago

Seems like there might be a bug in typings when resolving a relative link?

nickzelei commented 7 years ago

Hmm. I checked and I can't find where that file is being referenced anywhere. In fact, the path referenced does not exist.

And typings is able to successfully build the project when running

typings bundle -o test/bundle.d.ts

nickzelei commented 7 years ago

typings appears to think that the reference to ~ldapjs/lib/server should be resolved to lib/server/index, so it must think server is a folder, when it is in fact a module.

blakeembrey commented 7 years ago

I think it's worse that that, it searches for server.d.ts and falls back to server/index.d.ts to support the node.js-like use-cases. However, the issue appears to be the .. relative paths aren't going up a directory. It's odd, because I'm sure I have other libraries using this pattern - something must be off somewhere.

nickzelei commented 7 years ago

That's the same conclusion I arrived to as well. It's unable to go up a level. However, these type definitions are doing that in plenty of other places, which makes this error even more bizarre.

blakeembrey commented 7 years ago

Enabling debug, I see it landing in https://raw.githubusercontent.com/types/npm-ldapjs/4f8b8a7be5d422b5bed8ac9b4b5b74f8ce4e5064/lib/messages//message.d.ts (notice the two slashes, which is why it's messing up going up the directory). Not sure where those extra slashes are from yet.

blakeembrey commented 7 years ago

Here we are, try https://github.com/types/npm-ldapjs/blob/4f8b8a7be5d422b5bed8ac9b4b5b74f8ce4e5064/lib/messages/bind_response.d.ts#L1.

blakeembrey commented 7 years ago

The long term fix might be for typings to normalize these paths beforehand so we don't end up in this case.

nickzelei commented 7 years ago

Ahhh good catch.

With my limited knowledge of typings, it appears that it already accounts for this in some cases as during the bundle process it's able to resolve ..//result to ~ldapjs/lib/messages/result.

Regardless, thank you for tracking that down. I've submitted another PR upstream to fix the import.