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.

Sign up for GitHub

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

Typescript migration #113

Merged
merged 9 commits into from
Aug 19, 2020
Merged

Typescript migration #113

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7976676
Initial TS migration
wolfy1339 Apr 2, 2019
13f9b8f
TS Migration Pt2
wolfy1339 Apr 2, 2019
39d0497
Move createChannel into the class definition
wolfy1339 Apr 2, 2019
4caaf78
Convert tests to TypeScript
wolfy1339 Apr 2, 2019
64174e4
Add tooling
wolfy1339 Apr 2, 2019
3a136b0
Update ignore files
wolfy1339 Apr 2, 2019
df81262
Add missing type package for nock
wolfy1339 Apr 2, 2019
5d91128
build(package): npm audit fix
gr2m Aug 12, 2020
00ac3ae
types: Add stricter types from Probot for the Client options
wolfy1339 Aug 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,6 @@ logs.json
server/public/main.min.*
coverage
.vscode
index.js
*.d.ts
*.tsbuildinfo
3 changes: 3 additions & 0 deletions .npmignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
index.ts
tsconfig.json
tsconfig.tsbuildinfo
43 changes: 24 additions & 19 deletions index.js → index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
const validator = require('validator')
const EventSource = require('eventsource')
const superagent = require('superagent')
const url = require('url')
const querystring = require('querystring')
import validator = require('validator')
import EventSource = require('eventsource')
import superagent = require('superagent')
import url = require('url')
import querystring = require('querystring')

class Client {
constructor ({ source, target, logger = console }) {
source: string;
target: string;
logger: Console;
Copy link
Contributor

Choose a reason for hiding this comment

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

In probot we are more precise about this type:

https://github.com/probot/probot/blob/589f891feb51d18660db6b06707d886486b7d975/src/%40types/smee-client/index.d.ts#L4-L10

Shall we do the same here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, indeed. they should be updated to those ones

events!: EventSource;

constructor ({ source, target, logger = console }: { source: string, target: string, logger?: Console }) {
this.source = source
this.target = target
this.logger = logger
Expand All @@ -15,7 +20,13 @@ class Client {
}
}

onmessage (msg) {
static async createChannel () {
return superagent.head('https://smee.io/new').redirects(0).catch((err) => {
return err.response.headers.location
})
}

onmessage (msg: any) {
const data = JSON.parse(msg.data)

const target = url.parse(this.target, true)
Expand All @@ -24,7 +35,7 @@ class Client {

delete data.query

const req = superagent.post(target).send(data.body)
const req = superagent.post(url.format(target)).send(data.body)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you recall why you did this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't remember exactly.
But removing it causes errors

Argument of type 'UrlWithParsedQuery' is not assignable to parameter of type 'string'.


delete data.body

Expand All @@ -36,7 +47,7 @@ class Client {
if (err) {
this.logger.error(err)
} else {
this.logger.info(`${req.method} ${req.url} - ${res.statusCode}`)
this.logger.info(`${req.method} ${req.url} - ${res.status}`)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you recall why you did this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same comment as above.

Property 'statusCode' does not exist on type 'Response'.

}
})
}
Expand All @@ -45,15 +56,15 @@ class Client {
this.logger.info('Connected', this.events.url)
}

onerror (err) {
onerror (err: any) {
this.logger.error(err)
}

start () {
const events = new EventSource(this.source)
const events = new EventSource(this.source);

// Reconnect immediately
events.reconnectInterval = 0
(events as any).reconnectInterval = 0 // This isn't a valid property of EventSource

events.addEventListener('message', this.onmessage.bind(this))
events.addEventListener('open', this.onopen.bind(this))
Expand All @@ -66,10 +77,4 @@ class Client {
}
}

Client.createChannel = async () => {
return superagent.head('https://smee.io/new').redirects(0).catch((err, res) => {
return err.response.headers.location
})
}

module.exports = Client
export = Client
Loading