Skip to content

Fix function.apply type definition#22775

Merged
mhegazy merged 1 commit into
microsoft:masterfrom
adriengibrat:FixFunctionApplyDefinition
Mar 26, 2018
Merged

Fix function.apply type definition#22775
mhegazy merged 1 commit into
microsoft:masterfrom
adriengibrat:FixFunctionApplyDefinition

Conversation

@adriengibrat
Copy link
Copy Markdown
Contributor

@adriengibrat adriengibrat commented Mar 21, 2018

Fixes #22600

DanielRosenwasser
DanielRosenwasser requested changes Mar 22, 2018
View reviewed changes
Comment thread lib/lib.d.ts Outdated
* @param argArray A set of arguments to be passed to the function.
*/
apply(this: Function, thisArg: any, argArray?: any): any;
apply(this: Function, thisArg: any, argArray?: any[]): any;
Copy link
Copy Markdown
Member

@DanielRosenwasser DanielRosenwasser Mar 22, 2018

Choose a reason for hiding this comment

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

Is this compatible with IArguments? What about ReadonlyArray?

@DanielRosenwasser
Copy link
Copy Markdown
Member

DanielRosenwasser commented Mar 22, 2018

I think you really want a readonly-version of ArrayLike. In other words, instead of any[], you want Readonly<ArrayLike<any>>.

@DanielRosenwasser DanielRosenwasser mentioned this pull request Mar 22, 2018
Closed
@adriengibrat adriengibrat force-pushed the FixFunctionApplyDefinition branch from 1560213 to 35ee37c Compare March 22, 2018 19:41
@adriengibrat
Copy link
Copy Markdown
Contributor Author

adriengibrat commented Mar 22, 2018 •
edited
Loading

Thanks for your review, using Readonly<ArrayLike<any>> is indeed way better, done in 4f0a2e2

@adriengibrat adriengibrat force-pushed the FixFunctionApplyDefinition branch from 35ee37c to b068177 Compare March 25, 2018 20:42
@RyanCavanaugh
Copy link
Copy Markdown
Member

RyanCavanaugh commented Mar 26, 2018

@DanielRosenwasser 👍 ? CircleCI failures are unrelated

@adriengibrat
Copy link
Copy Markdown
Contributor Author

adriengibrat commented Mar 26, 2018 •
edited
Loading

I have rebased to upstream/master and push to run circle ci again.

@adriengibrat adriengibrat force-pushed the FixFunctionApplyDefinition branch from b068177 to 4f0a2e2 Compare March 26, 2018 21:12
@mhegazy mhegazy merged commit 36b49c0 into microsoft:master Mar 26, 2018
@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Mar 26, 2018

thanks @adriengibrat!

@microsoft microsoft locked and limited conversation to collaborators Jul 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@DanielRosenwasser DanielRosenwasser DanielRosenwasser requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@adriengibrat @DanielRosenwasser @RyanCavanaugh @mhegazy