Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disambiguation for load interface #518

Closed
wants to merge 1 commit into from
Closed

Disambiguation for load interface #518

wants to merge 1 commit into from

Conversation

eordano
Copy link

@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
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
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
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
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