Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Type definition for node-modern-rcon. #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

electricessence
Copy link

Basic external type declaration that currently reflects rcon.js.

@levrik: thank you, and you're welcome. 😁

Basic external type declaration that currently reflects `rcon.js`.
@electricessence
Copy link
Author

electricessence commented Apr 22, 2018

Just a thought, I'm not sure if you need to be exporting the RconError since it's only instantiated internally.

I welcome you to review my main fork as well. There's some adjustments including logging and send can be immediately queued. Event handling is refined to be properly stateful. The state property is more informative.

But fundamentally, the code you wrote is solid. Thanks again. I'm able to connect to Factorio successfully using Nodejs. :) I wonder how easy this would be to port to the browser. :|

@electricessence electricessence changed the title Type declaration for node-modern-rcon. Type definition for node-modern-rcon. Apr 22, 2018
@levrik
Copy link
Owner

levrik commented Apr 23, 2018

It's exported so people can check via instanceof

@electricessence
Copy link
Author

Oh yeah of course. Something I blanked on. :P

@electricessence
Copy link
Author

electricessence commented Apr 25, 2018

@levrik Ok all set. I added the last touch to the project file so it includes the typings and updated the patch version so it can be published.

@@ -0,0 +1,23 @@
/// <reference types="node" />
Copy link
Owner

Choose a reason for hiding this comment

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

Wasn't these reference comments removed from TypeScript?

Copy link
Author

Choose a reason for hiding this comment

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

As far as I know they still work. Might be deprecated.
It depends on your module structure.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants