Skip to content

Disambiguation for load interface#518

Closed
eordano wants to merge 1 commit intoprotobufjs:masterfrom
eordano:patch-1
Closed

Disambiguation for load interface#518
eordano wants to merge 1 commit intoprotobufjs:masterfrom
eordano:patch-1

Conversation

@eordano
Copy link
Copy Markdown

@eordano eordano commented Dec 5, 2016

This change allows the typechecker to know the type that is going to be returned.

This change allows the typechecker to know the type that is going to be returned.
@dcodeIO
Copy link
Copy Markdown
Member

dcodeIO commented Dec 5, 2016

Unfortunately, the .d.ts file is generated automatically through tsd-jsdoc and as thus needs a fix on jsdoc level.

Edit: Could you let me know of your specific use case? For me, this seems to work:

(protobuf.load("hello.proto") as Promise<protobuf.Root>).then((root) => {
    ...
});

dcodeIO added a commit that referenced this pull request Dec 6, 2016
…se or undefined, aliased Reader/Writer-as-function signature with Reader/Writer.create for typed dialects, see #518
@dcodeIO
Copy link
Copy Markdown
Member

dcodeIO commented Dec 6, 2016

Should work without type hints now (I believe nobody would use the non-promise return value anyway).

@dcodeIO dcodeIO closed this Dec 6, 2016
@eordano
Copy link
Copy Markdown
Author

eordano commented Dec 6, 2016

Hey, sorry for the late reply. Thanks for fixing this!

Using typescript 1.8 it complains about "Object" not having a then property. And I wouldn't like to make the as Promise... correction, too verbose :)

@dcodeIO
Copy link
Copy Markdown
Member

dcodeIO commented Dec 6, 2016

The fix isn't published on npm, yet, so you are probably still using a .d.ts with the Promise<Root> | Object return signature. The fix above changed the return signature to Promise<Root> | undefined which properly infers to Promise<Root> when appending .then(...).

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 this pull request may close these issues.

2 participants