Make the TypeScript types strict and better #849

LinusU · 2019-08-03T10:57:43Z

Blocked on: form-data/form-data#435 or DefinitelyTyped/DefinitelyTyped#37331

Next steps:

Upgrade to TypeScript 3.5
Make test/**/*.ts conform to strict mode
Turn on strict mode in tsconfig.json
Everything else from Make the TypeScript types strict and better #758

There is currently 1218 TypeScript errors in test files when running in strict mode 😂

Will submit some typings upstream and then type everything up!

source/utils/unhandle.ts

source/utils/types.ts

source/utils/timed-out.ts

LinusU · 2019-08-03T11:01:48Z

@sindresorhus as soon as form-data/form-data#435 or DefinitelyTyped/DefinitelyTyped#37331 is merged we could merge just this commit if you want to. It adds strict type checkin to the library part, and enforces it with an extra tsc call in the test phase.

Or we could wait for me to finish typing the tests ☺️

sindresorhus · 2019-08-03T11:14:19Z

// @vladfrangu FYI

sindresorhus · 2019-08-03T11:15:28Z

Or we could wait for me to finish typing the tests ☺️

I would prefer merging this when it passes first. Smaller PRs are good and makes it less likely to conflict with other changes.

package.json

source/utils/timed-out.ts

Co-Authored-By: Sindre Sorhus <sindresorhus@gmail.com>

LinusU · 2019-08-04T17:28:02Z

Running into microsoft/TypeScript#32693 when upgrading to TS 3.5 😬

vladfrangu

👍 from me, past these few questions/nits

vladfrangu · 2019-08-07T18:23:19Z

source/utils/timed-out.ts


-				if (socketPath || net.isIP(hostname || host)) {
+				if (socketPath || net.isIP(hostname || host || '')) {


I see that this net.isIP check is used at least twice, can't we abstract it to one central variable?

vladfrangu · 2019-08-07T18:26:32Z

source/utils/timed-out.ts

@@ -32,7 +35,7 @@ export default (request: ClientRequest, delays: Delays, options: TimedOutOptions
 	const cancelers: Array<typeof noop> = [];
 	const {once, unhandleAll} = unhandler();

-	const addTimeout = (delay: number, callback: (...args: unknown[]) => void, ...args: unknown[]): (typeof noop) => {
+	const addTimeout = <T extends any[]>(delay: number, callback: (delay: number, ...args: T) => void, ...args: T): (typeof noop) => {


Does the callback actually take the delay as the first argument? it doesn't seem like it

source/utils/timed-out.ts

sindresorhus · 2019-08-30T04:18:45Z

TypeScript 3.6 is out. Maybe that fixes some bugs? https://devblogs.microsoft.com/typescript/announcing-typescript-3-6/

szmarczak · 2019-09-14T10:27:18Z

@LinusU ping 😉

See #849 Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>

sindresorhus · 2019-09-17T07:54:17Z

I've pulled the current changes of this PR into master as I plan to do an early alpha release: 5002e54

sindresorhus · 2019-09-24T08:38:45Z

I have upgraded TS to latest now: 640c30b

yovanoc · 2019-09-24T08:59:53Z

Hello, you have to put global. before setInterval and setTimeout to handle NodeJS properly, thanks.

szmarczak · 2019-09-24T09:30:43Z

@yovanoc Why? There's no need.

yovanoc · 2019-09-24T09:34:14Z

Check yourself, with NodeJS setTimeout return a number and global.setTimeout return a NodeJS.Timeout

szmarczak · 2019-09-24T12:32:15Z

I did and you're wrong:

$ node
Welcome to Node.js v12.10.0.
Type ".help" for more information.
> setTimeout(() => {}, 1000);
Timeout {
  _idleTimeout: 1000,
  _idlePrev: [TimersList],
  _idleNext: [TimersList],
  _idleStart: 20031,
  _onTimeout: [Function],
  _timerArgs: undefined,
  _repeat: null,
  _destroyed: false,
  [Symbol(refed)]: true,
  [Symbol(asyncId)]: 98,
  [Symbol(triggerId)]: 5
}
> global.setTimeout(() => {}, 1000);
Timeout {
  _idleTimeout: 1000,
  _idlePrev: [TimersList],
  _idleNext: [TimersList],
  _idleStart: 43542,
  _onTimeout: [Function],
  _timerArgs: undefined,
  _repeat: null,
  _destroyed: false,
  [Symbol(refed)]: true,
  [Symbol(asyncId)]: 272,
  [Symbol(triggerId)]: 5
}

samhh · 2019-10-01T18:34:58Z

source/utils/timed-out.ts

@@ -91,7 +94,7 @@ export default (request: ClientRequest, delays: Delays, options: TimedOutOptions

 	if (delays.socket !== undefined) {
 		const socketTimeoutHandler = (): void => {
-			timeoutHandler(delays.socket, 'socket');
+			timeoutHandler(delays.socket!, 'socket');


Shouldn't the if statement act as a type guard, thus removing the need for the non-null assertion? Likewise in a few other places.

Already fixed in master: 640c30b#diff-7ab9a52ac2fccf434c1a34ce48f2883bL95-R95

Ah, my mistake! 👍

sindresorhus · 2019-11-01T08:25:11Z

Closing for lack of activity.

LinusU · 2019-11-01T10:28:17Z

Sorry for dragging my feet on this, closing is definitely the right approach 👍

Hopefully I'll get some time to work on this later ☺️

Make library code conform to strict type-checking

63638f5

LinusU commented Aug 3, 2019

View reviewed changes

source/utils/unhandle.ts Show resolved Hide resolved

LinusU commented Aug 3, 2019

View reviewed changes

source/utils/types.ts Show resolved Hide resolved

LinusU commented Aug 3, 2019

View reviewed changes

source/utils/timed-out.ts Show resolved Hide resolved

sindresorhus reviewed Aug 3, 2019

View reviewed changes

package.json Show resolved Hide resolved

sindresorhus reviewed Aug 3, 2019

View reviewed changes

source/utils/timed-out.ts Show resolved Hide resolved

sindresorhus reviewed Aug 3, 2019

View reviewed changes

source/utils/timed-out.ts Outdated Show resolved Hide resolved

Improve comment about helping TypeScript

72159fc

Co-Authored-By: Sindre Sorhus <sindresorhus@gmail.com>

LinusU mentioned this pull request Aug 4, 2019

Add TypeScript definitions lukechilds/create-test-server#36

Open

vladfrangu suggested changes Aug 7, 2019

View reviewed changes

LinusU mentioned this pull request Aug 27, 2019

Fix error in callback signatures form-data/form-data#435

Merged

sindresorhus added a commit that referenced this pull request Sep 17, 2019

Improve the TypeScript types

5002e54

See #849 Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>

samhh reviewed Oct 1, 2019

View reviewed changes

sindresorhus closed this Nov 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make the TypeScript types strict and better #849

Make the TypeScript types strict and better #849

LinusU commented Aug 3, 2019 •

edited by sindresorhus

Loading

LinusU commented Aug 3, 2019 •

edited

Loading

sindresorhus commented Aug 3, 2019

sindresorhus commented Aug 3, 2019

LinusU commented Aug 4, 2019

vladfrangu left a comment

vladfrangu Aug 7, 2019

vladfrangu Aug 7, 2019

sindresorhus commented Aug 30, 2019

szmarczak commented Sep 14, 2019

sindresorhus commented Sep 17, 2019

sindresorhus commented Sep 24, 2019

yovanoc commented Sep 24, 2019

szmarczak commented Sep 24, 2019

yovanoc commented Sep 24, 2019

szmarczak commented Sep 24, 2019

samhh Oct 1, 2019 •

edited

Loading

sindresorhus Oct 1, 2019

samhh Oct 1, 2019

sindresorhus commented Nov 1, 2019

LinusU commented Nov 1, 2019


		if (socketPath \|\| net.isIP(hostname \|\| host)) {
		if (socketPath \|\| net.isIP(hostname \|\| host \|\| '')) {

Make the TypeScript types strict and better #849

Make the TypeScript types strict and better #849

Conversation

LinusU commented Aug 3, 2019 • edited by sindresorhus Loading

LinusU commented Aug 3, 2019 • edited Loading

sindresorhus commented Aug 3, 2019

sindresorhus commented Aug 3, 2019

LinusU commented Aug 4, 2019

vladfrangu left a comment

Choose a reason for hiding this comment

vladfrangu Aug 7, 2019

Choose a reason for hiding this comment

vladfrangu Aug 7, 2019

Choose a reason for hiding this comment

sindresorhus commented Aug 30, 2019

szmarczak commented Sep 14, 2019

sindresorhus commented Sep 17, 2019

sindresorhus commented Sep 24, 2019

yovanoc commented Sep 24, 2019

szmarczak commented Sep 24, 2019

yovanoc commented Sep 24, 2019

szmarczak commented Sep 24, 2019

samhh Oct 1, 2019 • edited Loading

Choose a reason for hiding this comment

sindresorhus Oct 1, 2019

Choose a reason for hiding this comment

samhh Oct 1, 2019

Choose a reason for hiding this comment

sindresorhus commented Nov 1, 2019

LinusU commented Nov 1, 2019

LinusU commented Aug 3, 2019 •

edited by sindresorhus

Loading

LinusU commented Aug 3, 2019 •

edited

Loading

samhh Oct 1, 2019 •

edited

Loading