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

Some more node std libs #115

Merged
merged 4 commits into from
Nov 26, 2014
Merged

Conversation

jeffmo
Copy link
Contributor

@jeffmo jeffmo commented Nov 24, 2014

Adds a few more node module type signatures (Buffer, dgram, dns, domain, and events)

@jeffmo jeffmo changed the title Node std libs Some more node std libs Nov 24, 2014
declare module "dgram" {
declare function createSocket(type: string, callback?: () => void): void;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this return dgram$Socket?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch

@avikchaudhuri
Copy link
Contributor

Overlap with #108 on Buffer, how do we want to resolve this?

@jeffmo
Copy link
Contributor Author

jeffmo commented Nov 25, 2014

I'll remove my Buffer impl and comment on a few things I see in that one

@avikchaudhuri
Copy link
Contributor

OK, at first glance it seems you have more stuff in there, so I wouldn't just delete yours. Maybe try to merge?

@jeffmo
Copy link
Contributor Author

jeffmo commented Nov 25, 2014

I ran through and compared -- it looks like I actually got a couple things wrong (I verified any discrepancies I saw). Anyway, I commented on that PR with any issues I saw there -- so going to remove from here now

@@ -217,19 +217,149 @@ declare module "crypto" {
): void;
}

type dgram$Socket = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dgram.Socket inherits from EventEmitter

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh good catch, I'll circle back on this in a follow-up

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.

4 participants