Skip to content
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

Bug: incorrect tuple (array) type after changing in place #52375

Closed
antonilol opened this issue Jan 23, 2023 · 4 comments
Closed

Bug: incorrect tuple (array) type after changing in place #52375

antonilol opened this issue Jan 23, 2023 · 4 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@antonilol
Copy link

Bug Report

after you change an array in place (reverse, splice, push, pop, etc...) tuple types are unchanged and still reflect the type before the change

🔎 Search Terms

🕗 Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about _________

⏯ Playground Link

Playground link with relevant code

💻 Code

function getNumber(): [ string, number ] {
    if (Math.random() < 0.5) {
        return [ 'pi constant', Math.PI ];
    } else {
        return [ 'random number', Math.random() ];
    }
}

let a: number;

const s = getNumber();

a = s[1];

// @ts-expect-error
// [ string, number ]  ->  (string | number)[]
// Type 'string | number' is not assignable to type 'number'.
a = s.reverse()[0];

// @ts-expect-error
// After s got reversed in place, it is a [ number, string ], but typeof s = [ string, number ]
// Type 'string' is not assignable to type 'number'.
a = s[0];

🙁 Actual behavior

type is unchanged

🙂 Expected behavior

after Array.reverse() the type should update if the type specifies the order of the elements' types (like [string, number])

I also was playing around with a type that reverses a tuple type with some usage examples. this worked out pretty well, this could be useful for the Array.reverse type declaration, see the linked ts playground code

@RyanCavanaugh RyanCavanaugh added Working as Intended The behavior described is the intended behavior; this is not a bug Design Limitation Constraints of the existing architecture prevent this from being fixed and removed Working as Intended The behavior described is the intended behavior; this is not a bug labels Jan 23, 2023
@RyanCavanaugh
Copy link
Member

The in-place mutation methods for arrays aren't supported from a control flow perspective. I recommend using readonly [string, number] in most scenarios.

@jcalz
Copy link
Contributor

jcalz commented Jan 28, 2023

Cross-linking to #40316 and #48465

@antonilol
Copy link
Author

types can change because a different type is assigned (example), would it be possible to add some type annotation to class methods to tell what happens to the instance type?

@rotu
Copy link

rotu commented Dec 4, 2023

I don't think widening the declared type via control-flow-analysis is reasonable in the same way as narrowing.

But it is a problem that TypeScript allows operations that invalidate the underlying type annotations.

e.g. this is a type error:

let x: [number, string] = [1,'a']
x = [x[1], x[0]]

This is (almost) the same thing, but no type error:

let x: [number, string] = [1,'a']
x.reverse()

Non-readonly tuples are dangerously unsound. Given that this won't be fixed (per #6325), I'd avoid them completely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

4 participants