Skip to content

Array arguments to concat should be ReadonlyArrays #17806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
1 commit merged into from
Aug 16, 2017
Merged

Conversation

ghost
Copy link

@ghost ghost commented Aug 15, 2017

Fixes #17076
@sandersn You added the second overload in #10374, is there a reason we need two?

@ghost ghost force-pushed the readonly_concat branch from 714402a to d2ad4a3 Compare August 15, 2017 15:02
@sandersn
Copy link
Member

In inference, overloads are processed pairwise (if I recall correctly) so you need the same number of overloads for Array and ReadonlyArray to get correct inferences from each other.

@sandersn
Copy link
Member

In other words, if Array has two overloads, then ReadonlyArray needs two as well. Whether both are needed is a separate question, but I think if you look through the issue and PR history for Array.concat, you'll find that both are needed.

@ghost ghost merged commit 54af8aa into master Aug 16, 2017
@ghost ghost deleted the readonly_concat branch August 16, 2017 21:32
@ghost ghost mentioned this pull request Aug 23, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants