Convert index.js to TypeScript

[pbalaram]

With these changes, we can now use ES7 features.

class CloudSpeechRecognizer {
    constructor() {
        // code here
    }
}

I figure this might help keep things organized, as the codebase grows.

Bonus: You can now run examples/example.js by running the following.

npm run example

[FlorianWendelborn]

http://stackoverflow.com/questions/729692/why-should-text-files-end-with-a-newline

[ashishsc]

Thanks for the PR! This looks great, but I think we don't need classes. I'd rather move to using typescript and interfaces. That way we can get all the es7+ features and avoid classes

[pbalaram]

Sure, TypeScript makes sense. I've converted the core library to use it.
I'm not sure why we would avoid classes, would you mind explaining a bit further?

[evancohen]

Frankly I could go either way on this, but @ashishsc has strong feeling about code style that I typically defer to. Regardless, the one change I would request is that you include a prepublish script in package.json using

npm run build

That way I never accidentally publish the npm package without building the library first :)

[ashishsc]

we'll need tslint instead now.

[ashishsc]

Thanks for converting to typescript! we <3 you.

As far as I why I don't prefer classes. I like being able to pass the sonus object in as an easy way to mock the methods. Also this will be a breaking change.

However, I come from a functional programming background (internal state makes things harder to test, not that we have any) but @evancohen has more object-oriented experience so since he's fine with classes, by all means we should go with that.

[pbalaram]

Thank you for the feedback!

@evancohen I've added a prepublish step, which should call npm run build.
@ashishsc I've added tslint, fixed the trivial warnings, and added rules for the more widespread ones.

[evancohen]

This looks good to me. @ashishsc what say you?

[ashishsc]

Sorry one last change, we don't use semicolons, so if you could add the tslint semicolons:never rule, that would be sweet = ) https://palantir.github.io/tslint/rules/semicolon/

Nice job on this.

[pbalaram]

I'd be happy to add this, but I'm curious as to why this is the preference -- would you mind explaining a bit further?

If I understand correctly, the TypeScript compiler automatically inserts semi-colons where they are omitted in the TS source, yielding compiled JS output that always has semi-colon delimited statements.

This in turn introduces the possibility of issues such as microsoft/TypeScript#2575, where it's possible for the automatic insertion to adversely affect the runtime behavior of the code at hand.

Since this is the case, would it make more sense for us to avoid the risks, and require semi-colons?

For reference, this is also the recommended behavior from the TSLint recommended configuration.

https://github.com/palantir/tslint/blob/master/src/configs/recommended.ts

Thanks!

[ashishsc]

ah didn't realize that TS had these issues! Yes we should totally try and protect against these cases. LGTM in that case.

[ashishsc]

Although, it does look like they addressed that issue?microsoft/TypeScript#4788

[pbalaram]

Correct! They did resolve this particular issue, my intent was simply to suggest that we might be able to avoid this class of issues entirely by keeping semi-colons mandatory.

[ashishsc]

I think with the linter setting enabled we can already avoid these issues unless I'm mistaken?

…

On Sun, Mar 26, 2017, 7:30 PM Prithvi Balaram ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/sonus.ts <#35 (comment)>: > + threshold: 0, + device: this._device || null, + recordProgram: this._recordProgram || "rec", + verbose: false + }); + + this._mic.pipe(this._detector); + this._started = true; + } + + public stop() { + stopRecording(); + } + + public pause() { + this._mic.pause(); Correct! They did resolve this particular issue, my intent was simply to suggest that we might be able to avoid this class of issues entirely by keeping semi-colons mandatory. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#35 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACgRCtNWQm3pci1KjIvo9dH5rATC-GLsks5rpx80gaJpZM4MMSst> .

[evancohen]

Hey folks! I want to get this merged, but things were left in a bit of an ambiguous state. @ashishsc are we good on the linter settings?

[ashishsc]

I'm good with what we have currently, we can always change it later. We should just use an autoformatter and reduce bikeshedding at some point. Once pretter finishes their TS support, I'll add that in here.