feat(36327): libdef args rest type should always be array of any #36328

antongolub · 2020-01-21T11:31:40Z

Fixes #36327 36327

typescript-bot · 2020-01-21T11:31:43Z

It looks like you've sent a pull request to update our 'lib' files. These files aren't meant to be edited by hand, as they consist of last-known good states of the compiler and are generated from 'src'. Unless this is necessary, consider closing the pull request and sending a separate PR to update 'src'.

sandersn

Please move your changes to src/lib.

Additionally, it looks like you closed the source bug, so maybe this PR should be closed too.

antongolub · 2020-02-03T20:24:53Z

@sandersn

My issue was related to another reason, so I closed it. But I still suggest to describe ...args rest param as any[] for consistency.

sandersn

Changing only the constraint type, as Parameters and ConstructorParameters do, should get rid of the diff in inferTypes1.types. It's not a big diff, but it's nicer to avoid any changes at all.

sandersn · 2020-02-04T22:54:20Z

src/lib/es5.d.ts


 /**
 * Obtain the return type of a function type
 */
-type ReturnType<T extends (...args: any) => any> = T extends (...args: any) => infer R ? R : any;
+type ReturnType<T extends (...args: any[]) => any> = T extends (...args: any[]) => infer R ? R : any;


Suggested change

type ReturnType<T extends (...args: any[]) => any> = T extends (...args: any[]) => infer R ? R : any;

type ReturnType<T extends (...args: any[]) => any> = T extends (...args: any) => infer R ? R : any;

sandersn · 2020-02-04T22:54:29Z

src/lib/es5.d.ts


 /**
 * Obtain the return type of a constructor function type
 */
-type InstanceType<T extends new (...args: any) => any> = T extends new (...args: any) => infer R ? R : any;
+type InstanceType<T extends new (...args: any[]) => any> = T extends new (...args: any[]) => infer R ? R : any;


Suggested change

type InstanceType<T extends new (...args: any[]) => any> = T extends new (...args: any[]) => infer R ? R : any;

type InstanceType<T extends new (...args: any[]) => any> = T extends new (...args: any) => infer R ? R : any;

sandersn · 2020-02-04T22:56:38Z

@typescript-bot run dt

typescript-bot · 2020-02-04T22:56:43Z

Heya @sandersn, I've started to run the parallelized Definitely Typed test suite on this PR at a3bbc2e. You can monitor the build here. It should now contribute to this PR's status checks.

DanielRosenwasser · 2020-02-04T23:43:17Z

Is this change going to handle ReadonlyArray and the like correctly? Does it exhibit any different behavior when inferring from two overloads, one which is a subtype of any[] and one which is only assignable to any[]?

antongolub · 2020-02-05T10:56:36Z

@sandersn
Done.

@DanielRosenwasser
I'm afraid I can’t clarify this. My concern was just about custom IConstructable interfaces to strictly comply InstanceType internal constraint.

sandersn · 2020-02-05T17:22:05Z

@antongolub can you add tests for the cases @DanielRosenwasser pointed out?
@DanielRosenwasser mind giving examples, particularly of the two-overload case?

sandersn · 2020-02-12T22:44:49Z

@typescript-bot run dt

maybe it will work this time

typescript-bot · 2020-02-12T22:44:53Z

Heya @sandersn, I've started to run the parallelized Definitely Typed test suite on this PR at 38e6d5b. You can monitor the build here. It should now contribute to this PR's status checks.

sandersn · 2020-02-12T23:48:04Z

@typescript-bot @weswigham DT is (almost) clean but it did not contribute to the PR's status checks that I can see.

weswigham · 2020-02-13T00:36:21Z

Uhhh, yeah, the whole "contributes to status checks" bit was broken when we swapped to using the merge commit a few weeks ago instead of the branch head, because devops is bad. Should we swap back? IMO, forcing manual merges is probably worth the correct status linkage?

sandersn · 2020-02-19T21:03:11Z

Not contributing to branch status is a feature rather than a bug given how broken our suite 2 tests usually are. As long as it's a known feature I'm fine with it, and I think it's better to keep perf testing the correct commit.

sandersn · 2020-03-03T23:50:54Z

@DanielRosenwasser filed #37193 with 2 examples that behave strangely with the current definitions. We need to figure out why they behave badly before moving forward with this PR. (Checking the new definitions would be interesting too, but I suspect that they behave the same.)

sandersn · 2020-03-17T21:54:05Z

This PR hasn't seen any activity for quite a while, so I'm going to close it to keep the number of open PRs manageable. Feel free to open a fresh PR or continue the discussion here.

typescript-bot added the lib update PR modifies files in the `lib` folder label Jan 21, 2020

antongolub requested a review from DanielRosenwasser January 21, 2020 14:21

sandersn requested changes Feb 3, 2020

View reviewed changes

antongolub force-pushed the master branch 2 times, most recently from 1ae7a07 to ec983d8 Compare February 3, 2020 20:17

antongolub force-pushed the master branch from ec983d8 to a3bbc2e Compare February 3, 2020 20:43

antongolub changed the title ~~fix(36327): libdef args rest type should always be array of any~~ feat(36327): libdef args rest type should always be array of any Feb 3, 2020

sandersn requested changes Feb 4, 2020

View reviewed changes

feat(lib/es5): args rest type should always be array of any

38e6d5b

antongolub force-pushed the master branch from a3bbc2e to 38e6d5b Compare February 5, 2020 10:16

sandersn closed this Mar 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(36327): libdef args rest type should always be array of any #36328

feat(36327): libdef args rest type should always be array of any #36328

antongolub commented Jan 21, 2020

typescript-bot commented Jan 21, 2020

sandersn left a comment

antongolub commented Feb 3, 2020 •

edited

Loading

sandersn left a comment

sandersn Feb 4, 2020

sandersn Feb 4, 2020

sandersn commented Feb 4, 2020

typescript-bot commented Feb 4, 2020 •

edited

Loading

DanielRosenwasser commented Feb 4, 2020 •

edited

Loading

antongolub commented Feb 5, 2020

sandersn commented Feb 5, 2020

sandersn commented Feb 12, 2020

typescript-bot commented Feb 12, 2020 •

edited

Loading

sandersn commented Feb 12, 2020

weswigham commented Feb 13, 2020

sandersn commented Feb 19, 2020

sandersn commented Mar 3, 2020

sandersn commented Mar 17, 2020

	type ReturnType<T extends (...args: any[]) => any> = T extends (...args: any[]) => infer R ? R : any;
	type ReturnType<T extends (...args: any[]) => any> = T extends (...args: any) => infer R ? R : any;

	type InstanceType<T extends new (...args: any[]) => any> = T extends new (...args: any[]) => infer R ? R : any;
	type InstanceType<T extends new (...args: any[]) => any> = T extends new (...args: any) => infer R ? R : any;

feat(36327): libdef args rest type should always be array of any #36328

feat(36327): libdef args rest type should always be array of any #36328

Conversation

antongolub commented Jan 21, 2020

typescript-bot commented Jan 21, 2020

sandersn left a comment

Choose a reason for hiding this comment

antongolub commented Feb 3, 2020 • edited Loading

sandersn left a comment

Choose a reason for hiding this comment

sandersn Feb 4, 2020

Choose a reason for hiding this comment

sandersn Feb 4, 2020

Choose a reason for hiding this comment

sandersn commented Feb 4, 2020

typescript-bot commented Feb 4, 2020 • edited Loading

DanielRosenwasser commented Feb 4, 2020 • edited Loading

antongolub commented Feb 5, 2020

sandersn commented Feb 5, 2020

sandersn commented Feb 12, 2020

typescript-bot commented Feb 12, 2020 • edited Loading

sandersn commented Feb 12, 2020

weswigham commented Feb 13, 2020

sandersn commented Feb 19, 2020

sandersn commented Mar 3, 2020

sandersn commented Mar 17, 2020

antongolub commented Feb 3, 2020 •

edited

Loading

typescript-bot commented Feb 4, 2020 •

edited

Loading

DanielRosenwasser commented Feb 4, 2020 •

edited

Loading

typescript-bot commented Feb 12, 2020 •

edited

Loading