[node] Fix typings for various child_process methods #28025
Conversation
typescript-bot
added
Popular package
|
@thw0rted Thank you for submitting this PR! 🔔 @segayuu @parambirs @tellnes @WilcoBakker @octo-sniffle @smac89 @Flarna @mwiktorczyk @wwwy3y3 @DeividasBakanas @kjin @alvis @eps1lon @Hannes-Magnusson-CK @jkomyno @ajafff @hoo29 @n-e @BrunoScheufler @mohsen1 @KSXGitHub @a-tarasyuk @islishude @r3nya @ZaneHannanAU - please review this PR in the next few days. Be sure to explicitly select If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead. |
types/node/index.d.ts
Outdated
| keepOpen?: boolean; | ||
| } | ||
|
|
||
| export type StdioOptions = "pipe" | "ignore" | "inherit" | Array<("pipe" | "ipc" | "ignore" | stream.Stream | number)>; |
There was a problem hiding this comment.
null and undefined are also allowed as types in the array
There was a problem hiding this comment.
Good point, I overlooked those in the docs.
typescript-bot
added
The Travis CI build failed
and removed
Awaiting reviewer feedback
labels
|
@thw0rted The Travis CI build failed! Please review the logs for more information. Once you've pushed the fixes, the build will automatically re-run. Thanks! |
| export function fork(modulePath: string, args?: ReadonlyArray<string>, options?: ForkOptions): ChildProcess; | ||
|
|
||
| export interface SpawnSyncOptions { | ||
| argv0?: string; |
There was a problem hiding this comment.
I'm quite sure that argv0 argument is also supported for sync even if it is missing in node docs.
There was a problem hiding this comment.
It does seem pretty unlikely that spawnsync wouldn't end up reusing most of the logic from spawn (or vice versa). I'll put the option back here and see about getting the Node docs fixed -- if it turns out you really can't, I'll open a separate PR to take it out, I guess.
There was a problem hiding this comment.
That didn't take long: found it. Definitely uses the same logic for both functions.
typescript-bot
added
the
Owner Approved
|
@thw0rted The Travis CI build failed! Please review the logs for more information. Once you've pushed the fixes, the build will automatically re-run. Thanks! |
ghost
merged commit 
3 participants
Please fill in this template.
npm test.)npm run lint package-name(ortscif notslint.jsonis present).Select one of these and delete the others:
If changing an existing definition:
tslint.jsoncontaining{ "extends": "dtslint/dt.json" }.Required minimal changes outside of Node. Build appears to be broken but it's TS@next's fault as far as I can tell.