Skip to content

Implementation of concat gives unexpected results #39241

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

Closed
DanielRosenwasser opened this issue Jun 24, 2020 · 5 comments · Fixed by #39249
Closed

Implementation of concat gives unexpected results #39241

DanielRosenwasser opened this issue Jun 24, 2020 · 5 comments · Fixed by #39249
Assignees
Labels
Bug A bug in TypeScript

Comments

@DanielRosenwasser
Copy link
Member

function concat1<T extends readonly any[], U extends readonly any[]>(arr1: T, arr2: U): [...T, ...U] {
    return [...arr1, ...arr2];
}

function concat2<T extends readonly any[], U extends readonly any[]>(arr1: T, arr2: U) {
    return [...arr1, ...arr2];
}

const x1 = concat1([1,2,3] as const, [4,5, 6] as const);
const x2 = concat2([1,2,3] as const, [4,5, 6] as const);

Here, TypeScript is okay with concat1 returning [...T, ...U], but instead infers the type of concat2 to be T[number][].

Notice U was not in the output type!

As a result, x1 has the type [1, 2, 3, 4, 5, 6] and x2 has the type (1, 2, 3)[]).

@DanielRosenwasser
Copy link
Member Author

Feels strange to contrast with tail which works as one would expect:

// returns 'T'
function tail<T extends any[]>(arg: [any, ...T]) {
    const [_, ...result] = arg;
    return result
}

@DanielRosenwasser
Copy link
Member Author

@ahejlsberg any thoughts on this one?

@ahejlsberg
Copy link
Member

Yeah, inferring T[number][] instead of (T[number] | U[number])[] definitely isn't good. We actually do ask for a union of those two types, but sadly subtype reduction reduces it to T[number][]. The issue is the rule we have that says a type is assignable to T[K] if it is assignable to the base constraint of T[K] for writing. And the base constraint of T[K] for both reading and writing is any. The rule is unsound (for sure), but we have it to allow assignment of non-generic types to type this['foo'] in classes. I'm thinking we should restrict it to not kick in for index signatures in T.

@ahejlsberg
Copy link
Member

BTW, the fact that we infer an array type for concat2 is by design, it doesn't have a tuple contextual type nor is it as const. Not clear that we could easily change that.

@ahejlsberg
Copy link
Member

Actually, there's really no reason the unsound rule should be in effect for anything but the assignable and comparable relations. So, I think the fix is to disable it for the subtype relations, which causes us to preserve the full (T[number] | U[number])[]. I will put up a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants