microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.27k stars 12.39k forks source link

Omitting a property from tuple breaks destruction #54371

Open so1ve opened 1 year ago

so1ve commented 1 year ago

Bug Report

πŸ”Ž Search Terms

tuple omit property Searched in Bing and Github issues but I found nothing

πŸ•— Version & Regression Information

⏯ Playground Link

Link

πŸ’» Code

const foo = [1, "2"] as const
const [a, b] = foo // everything works well, a is number and b is string

const bar = [1, "2"] as Omit<[number, string], "at">
const [c, d] = bar // Oops! c and d are string | number now

πŸ™ Actual behavior

↑ c and d are both string | number

πŸ™‚ Expected behavior

a and b should have correct type

Peeja commented 1 year ago

Here's a playground link for you.

so1ve commented 1 year ago

Oh, thank you!

Peeja commented 1 year ago

Hmm, that's not the behavior I'm seeing. I'm getting individual types for the members of the tuple.

That said, you still can't get rid of toString, because it's on Object itself. :) But you can get rid of Array properties, like forEach, as demonstrated in that Playground.

so1ve commented 1 year ago

Ah I think I'm providing a wrong reproduction. I'll update it later.

so1ve commented 1 year ago

Could you please checkout https://github.com/so1ve/cride/blob/main/src/index.ts? Test the following code:

const foo = cride({}, [1, "2"])

This works well when you destruct it like const [a, b] = foo, but once you change the type to:

type IsomorphicDestructurable<
  T extends Record<string, unknown>,
  A extends readonly any[],
> = Pick<
  T & A,
  keyof T | (IsTuple<A> extends true ? NumberKeys<A> : `${number}`)
> &
  Omit<A, "at" | "slice">; // here!

It breaks.

so1ve commented 1 year ago

weird, it still works...

fatcerberus commented 1 year ago

@Peeja One curious thing I noticed is this: https://www.typescriptlang.org/play?ts=5.0.4#code/PTAEBcHtIG3BLADgZ1AQwE4FMIAsfJoC2OAZpBqACZYDGM6AdlaAK7JYBQtkjy4ocpABMoALygA2gEYANKABEwhQF10qHn3Dde-KWmHyARsLUShwgNycQoO3YB6Afk72DNsM84eI0OElQqeFJSLEojLHAAdywsRmo6BjRmNg4dLUFoAGZxKTlFZTU0VAB5InhwAB4ZeSVVWvIMAFE0WlwFAD50vUk0LOMss0zILOtbe1AvNyyfLyA

For the Omit case, the type hints differ depending on where you hover (declaration vs. use), whereas they're the same for a virgin tuple type. So there's something different happening with type inference between the two cases that could be causing issues in more complex scenarios.

so1ve commented 1 year ago

Ah yes! That's a little bit like what I'm facing. Interesting that both my declaration and use are wrongly typed.

Peeja commented 1 year ago

@fatcerberus Huh, that is odd... πŸ˜•

so1ve commented 1 year ago

A new playground: Link

so1ve commented 1 year ago

It seems that mapping type will break the tuple type.

fatcerberus commented 1 year ago

@so1ve That's just the same thing I already pointed out above: declaration site shows string | number, later uses have the proper type: Playground

so1ve commented 1 year ago

Link

In this example you can see that a and c is typed correctly but reject and resolve is not.

Peeja commented 1 year ago

Found it! The issue seems to happen when one of the tuple type members is assignable to the other. For instance:

declare const bar: Omit<[number, 1], "at">
const [c, d] = bar
//     ^?
   c;
// ^? number

// ❌ This should be `1`
   d;
// ^? number

Instead of 1, d was typed as number, which I would assume TS is reducing from number | 1. It's easier to see when the elements are functions:

declare const bar: Omit<[(x: number) => void, (x: 1) => void], "at">
const [c, d] = bar
//     ^?

// ❌ This should be `(x: number) => void`
   c;
// ^? (x: number) => void | (x: 1) => void

   d;
// ^? (x: 1) => void

And that kind of makes sense: here, c is typed as accepting either a number or specifically 1; the latter type is just redundant. In fact, if you give them different return types so it's not redundant, you actually get the right type and not a union.


…And it's at this point in the writeup that I discover the actual problem (sort of). All you need to do to "fix" this is to change your target to ES2021 or earlier. This problem only appears to occur while targeting ES2022 and ESNext. I have no idea if that's intentional. My gut feeling is that it's a bug, because it seems to behave in ways that can't be right, but I suppose it's possible TS is just expressing something that's new in ES2022 that I'm not thinking of. (I've been surprised before…)

Here's the issue reduced to something actually incorrect. Bizarrely, the type of the value changes depending on whether it's called as a function: when it's used in a function call, it picks of the union members, but it somehow picks the wrong one.

declare const bar: Omit<[(x: number) => void, (x: 1) => void], "at">
const [c, d] = bar
//     ^?

// ❌ This should be `(x: number) => void`
   c;
// ^?

// ❌ This should also be `(x: number) => void`, and it should accept a 2
   c(2);
// ^?

   d;
// ^?
fatcerberus commented 1 year ago

This problem only appears to occur while targeting ES2022 and ESNext. I have no idea if that's intentional. My gut feeling is that it's a bug, because it seems to behave in ways that can't be right, but I suppose it's possible TS is just expressing something that's new in ES2022 that I'm not thinking of.

This seems unlikely. Tuple types are purely a TS concept; they are plain old arrays at runtime. It seems more likely that something else changed in type inference that causes this indirectly.

RyanCavanaugh commented 1 year ago

In user code, I'd recommend using a newer version of Omit that is homomorphic:

type MappedOmit<T, K> = { [P in keyof T as P extends K ? never : P]: T[P] };

But yeah the behavior describes seems a little off

Andarist commented 1 year ago

This isArrayLikeType(parentType) check is at fault here: https://github.dev/microsoft/TypeScript/blob/fbd63e9e43dc4e64184b590fa156002a7be6d47e/src/compiler/checker.ts#L10376

I'm not quite sure how this should be adjusted though. Currently isArrayLikeType is defined like this:

    function isArrayLikeType(type: Type): boolean {
        // A type is array-like if it is a reference to the global Array or global ReadonlyArray type,
        // or if it is not the undefined or null type and if it is assignable to ReadonlyArray<any>
        return isArrayType(type) || !(type.flags & TypeFlags.Nullable) && isTypeAssignableTo(type, anyReadonlyArrayType);
    }

In general, tuples and arrays are not always handled in the compiler in various scenarios - one of other things that comes to my mind is that intersections with tuple/array members are not always handled as expected (although that works in this particular scenario). So perhaps a fix here should involve a more general overhaul of how those things are treated within the compiler instead of being an ad-hoc change to satisfy this particular scenario.