Destructuring index signature by computed property loses type information

[amirburbea]

TypeScript Version: 2.4.2

Code

type Zomg = { x: number };
type Foobar = { foo: number, bar: Zomg };

const test: { [key: string]: Foobar } = {
  a: { foo: 1, bar: { x: 2 } },
  b: { foo: 3, bar: { x: 4 } },
};

// TSC does not complain about this because destructuring by [key] erroneously makes the compiler infer the type as 'any'.
let key = 'b'; // tried with const and let - results were identical
const { [key]: { bar: { x: x1, y: y1 } } } = test;

// Error: type 'Zomg' has no property 'y' and no string index signature. 
const { ['b']: { bar: { x: x2, y: y2 } } } = test;

Expected behavior:
Both examples (destructure by [key] and destructure by ['b']) should yield identical behavior - in this case an error because there is no y in type Zomg

Actual behavior:
Example 1 is allowed to proceed and more importantly loses all type information about the destructured variables (so x1 is any instead of number whereas x2 correctly is inferred as a number).

[DanielRosenwasser]

You've said that test has an index signature; you're basically ignoring properties on the RHS and guaranteeing that any property on test is a FooBar.

[amirburbea]

This is NOT working as intended. If every property on test is a Foobar, then it should not destructure the right as any but as a Foobar. But as you can see, when passed an arbitrary string key it forgets that and treats the right side as any.

TSC handles example 2 correctly but example one incorrectly. That's the problem. (my real use case is more of a deep structure where this constantly requires me to declare additional variables in order to access the structures I care about).

[amirburbea]

Please remove the label 'working as intended' as it most certainly is not. As said, if it was, the behaviors would be identical and they're not.

…

On Wed, Aug 2, 2017, 12:16 PM Daniel Rosenwasser ***@***.***> wrote: You've said that test has an index signature; you're basically ignoring properties on the RHS and guaranteeing that any property on test is a FooBar. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#17566 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACBnil70JVXXWXLTZiBBwyhp8eSiQ6C9ks5sUK7dgaJpZM4OrSZo> .

[RyanCavanaugh]

@amirburbea providing a simpler example would be more useful than demanding that we apply our triage process differently

@DanielRosenwasser I don't think this is correct? Example

type Points = { [key: string]: { x: number, y: number } };
const Points: Points = <any>null;
const { ['a']: { x: var_x1, y: var_y1, z: var_z1 } } = Points;
const A: 'a' = 'a';
const { [A]: { x: var_x2, y: var_y2, z: var_z2 } } = Points;

[amirburbea]

Thank you @RyanCavanaugh (and my sincere apologies to you and @DanielRosenwasser if I went about this the wrong way). Your example does perfectly illustrate the same issue - property z is not in Points and indexing by 'a' is flagging it correctly but indexing by A even when A is a const limited to 'a' is not flagging the error.

[amirburbea]

@RyanCavanaugh, I hate to press but seeing as the 'Working as Intended' label is still on the ticket, should I be worried that this bug won't get addressed?

The bug is displayed in an even worse manner in the sample below.

const foobar = {} as { [key: string]: { zomg: number } };
const { ['l']: { zomg: a1 } } = foobar;
// test1 is correctly typed as a number - attempting to assign something else than a number would not be allowed.
const test1: typeof a1 = 6;
const { [test1.toString()]: { zomg: a2 } } = foobar;
// test2 is incorrectly typed as any - allowing me to assign a string even though it should not.
const test2: typeof a2 = '';

[amirburbea]

any progress? or do you want to tell me where I should start looking if i'd want to perhaps investigate this myself as my first PR?

[sandersn]

No, I haven't had a chance to look at this one.

You could start by looking at checkVariableLikeDeclaration in checker.ts. A couple of clauses down is a check for BindingElements (an example from above: [A]: { ... }), and inside that is a call to checkComputedPropertyName. The bug could be there, or it could be later in checkVariableLikeDeclaration; there are a couple of sections that check the whole binding pattern.

[levi217]

Any updates on this? Running into same issue.

[sandersn]

@levi217 my advice for fixing this is the same as to @amirburbea.

[Andarist]

@RyanCavanaugh this issue can be closed, it was fixed long time ago by #23489