Add various effective UID and groups operations #21256
Conversation
|
types/node/index.d.ts to authors (@DefinitelyTyped/DefinitelyTyped @parambirs @tellnes @WilcoBakker @octo-sniffle @smac89 @Flarna @mwiktorczyk @wwwy3y3 @DeividasBakanas @kjin @alvis Microsoft TypeScript (account can't be detected)). Could you review this PR? |
RyanCavanaugh
added
the
Popular package
types/node/index.d.ts
Outdated
| @@ -477,6 +477,14 @@ declare namespace NodeJS { | |||
| getuid(): number; | |||
| setuid(id: number): void; | |||
| setuid(id: string): void; | |||
| geteuid(): number; | |||
| seteuid(id: number): void; | |||
| seteuid(id: string): void; | |||
There was a problem hiding this comment.
Please use an union type instead an overload, e.a. seteuid(id: string | number): void.
Would be nice if you could adapt the nearby functions setgid and setuid also...
There was a problem hiding this comment.
Why is a union type better than an overload?
There was a problem hiding this comment.
It's the way how typescript does the lookup and type detection. If you use an union type it's one function with one signature, if you use overloads then they are two functions.
Following does not transpile currently but works if union types are used:
function foo<T>(f: (p: T) => void, p: T) {
return f(p);
}
foo(process.setgid, "foo"); // OK
foo(process.setgid, 5); // Argument of type '5' is not assignable to parameter of type 'string'
Typescript select always the last overload in such situations.
types/node/index.d.ts
Outdated
| seteuid(id: number): void; | ||
| seteuid(id: string): void; | ||
| getegid(): number; | ||
| setegid(id: number): void; |
There was a problem hiding this comment.
Same as for seteuid, please use an union type instead an overload.
types/node/index.d.ts
Outdated
| setegid(id: number): void; | ||
| setegid(id: string): void; | ||
| getgroups(): number[]; | ||
| setgroups(groups: (string | number)[]): void; |
There was a problem hiding this comment.
I think you have to write Array<string | number> to keep lint happy.
|
@jez9999 Please fix the failures indicated in the Travis CI log. |
types/node/index.d.ts
Outdated
| @@ -13,7 +13,7 @@ | |||
| // Deividas Bakanas <https://github.com/DeividasBakanas> | |||
| // Kelvin Jin <https://github.com/kjin> | |||
| // Alvis HT Tang <https://github.com/alvis> | |||
| // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped | |||
| // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped) | |||
There was a problem hiding this comment.
I expect this change was not intended.
|
I changed it to unions. |
|
@Flarna @RyanCavanaugh Can these be merged now? |
|
@DanielRosenwasser Can this be merged please? |
|
@jez9999 Please fix the failures indicated in the Travis CI log. |
|
I don't think that failure has anything to do with my changes. |
|
@andy-ms @ianfp @DanielRosenwasser @andypyrope @evanshortiss Somebody please merge this in! Otherwise my code using Process breaks unless i hack it temporarily. And this project needs more people reviewing/checking stuff in :-\ |
|
@jez9999 not sure why you've mentioned me, I'm not an org member with merge rights. Maybe a mistake? |
|
It said you merged this: 02fc61f |
|
Approved by a listed owner. PR ready to merge pending express review by a maintainer. |
|
Luckily it's been merged now. |
Add various effective UID and groups operations
Addresses issue #21114.