Skip to content

Cannot assign property to same type with generic key #32693

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

Open
LinusU opened this issue Aug 3, 2019 · 11 comments
Open

Cannot assign property to same type with generic key #32693

LinusU opened this issue Aug 3, 2019 · 11 comments
Labels
Experience Enhancement Noncontroversial enhancements Suggestion An idea for TypeScript
Milestone

Comments

@LinusU
Copy link
Contributor

LinusU commented Aug 3, 2019

TypeScript Version: 3.6.0-dev.20190803

Search Terms: assigning keyof generic key

Code

type A = { a: number }
type B = { b: number }

interface Foo {
    a: A
    b: B
}

declare let target: Foo
declare let source: Foo
declare let key: keyof Foo

target[key] = source[key]

Expected behavior:

I expect it to allow the assignment. Since typeof target and typeof source are the same, I expect typeof target[key] to always be the same as typeof source[key].

Actual behavior:

error TS2322: Type 'A | B' is not assignable to type 'A & B'.

Playground Link: https://www.typescriptlang.org/play/#code/C4TwDgpgBAglC8UDeUCGAuKA7ArgWwCMIAnKAXwChRIoAhBZKAzXQk8iigSy2BIDNUAY2gAxAPbjkFKLLSYYMuczoVKFACYQhAG1TFoOiMCjB9Ac2OYJ4zdr0GoRkwGdxOYiOuS7u-YeMoAGsIEEwQkHF+KBtOM2JLYABtCIBdBjcPERTQ1IogA

Related Issues: #31665 (but it was closed and the comments seem to indicate that this should work 🤔) ping @RyanCavanaugh

@fatcerberus
Copy link

See #30769 and #31445. The compiler only sees the types when checking the assignment and doesn't realize that [key] accesses the same property on both sides, and therefore can't prove the assignment is safe. As far as TS is concerned you might be doing target.b = source.a (or vice versa).

This used to work prior to #30769 (TS 3.5), but only as a consequence of being unsound in general:

type A = { a: number }
type B = { b: number }

interface Foo {
    a: A
    b: B
}

declare let target: Foo
declare let source: Foo
declare let k1: keyof Foo
declare let k2: keyof Foo

// this was incorrectly allowed before TS 3.5
target[k1] = source[k2]

@jack-williams
Copy link
Collaborator

I think the title is slightly misleading here: the key in your example is not generic and #30769 specifically does not affect assignments where the key is generic.

function assignProp<K extends keyof Foo>(target: Foo, source: Foo, key: K) {
    target[key] = source[key]; // no error
}

@fatcerberus
Copy link

In a sense keyof itself could be seen as generic, insofar as it’s a type which is parameterized on another type. But you’re right that it’s not “a generic key” in the sense that we would normally use the term. 😄

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Aug 5, 2019

#31445 is the canonical one for this problem.

That said, I think we should revisit since it seems to be coming up reasonably often. Special-casing a[k] = b[k] for identical identifiers k would fix 95% of these and in fact it's hard to imagine a sound assignment where the ks differed

@RyanCavanaugh RyanCavanaugh added In Discussion Not yet reached consensus Suggestion An idea for TypeScript labels Aug 5, 2019
@bre1470
Copy link

bre1470 commented Aug 7, 2019

Yes, please consider special case handling. In our project we have to stick on 3.4.5, because TypeScript does not get the following pattern anymore:

interface O {
    a?: (object|string);
    b?: string;
}

function B (p?: ('a'|'b')): void {
    var o: O = {};
    if (p) {
        o[p] = o[p] || {};
    }
}

@hjkcai
Copy link

hjkcai commented Aug 21, 2019

CC @medns

@RyanCavanaugh RyanCavanaugh added Experience Enhancement Noncontroversial enhancements and removed In Discussion Not yet reached consensus labels Mar 13, 2020
@RyanCavanaugh
Copy link
Member

Approved for the case where

t[x] = s[x]

where x is an "identical reference" (the same function used for CFA). When this happens, relate the assignment using the old "union target" rules about the left-hand side instead of the newer stricter "intersection target" put in place. This would be a special case in checkBinaryExpression. Ping me for clarification if needed.

@k4hvd1
Copy link

k4hvd1 commented Apr 5, 2020

related to explanation from @RyanCavanaugh

Your code makes sense, but it is a different case from the one I submitted.

You proposed general case and x[g] is not assignable to x[f]. However its a bit tricky and special case should be considered. So, if the indexer is the same type than operation should be allowed.
The type of indexer is not just its declared type, perhaps it also depends on exact instance. This is not currently checked with compiler, and this raise error for correct code , please check the code.

type SimpleType = { a: string, b: number }
let x,y: SimpleType = { a: "", b: 9 }
let fields: (keyof SimpleType)[] = ["a", "b"]
for (const g of fields) {
    for (const f of fields) {
        x[f] = x[g] //OK (Error -> and should not be allowed; type are not same: each one is string|number )
        y[f] = x[f] //Error -> but is OK (types are the same: each one is string or each one is number)
        x[f] = x[f] //Error -> but is OK (types are the same)

        let h = (Math.random() > 0.5) ? fields[0] : fields[1]
        y[h] = x[x] //Should be an error
        let j = f
        y[j]=x[f] //Would be nice if the complier let this compile to
  }
}

code link

Proposal:

  1. add additional check while compiling that checks both type and variable in indexer

  2. add special keyword (to simplify and limit compiling effort)

  for (const f of /*keyword_of_mapped_field*/ fields)

and implement additional indexer instance check

  1. add special keyword (to simplify and limit compiling effort)
	y[/*keyword_of_mapped_indexer(*/f/*)*/]=  y[/*keyword_of_mapped_indexer(*/g/*)*/]

and implement additional indexer instance check

@ericdrobinson
Copy link

ericdrobinson commented Jan 6, 2022

Very much looking forward to the special case getting implemented at some point. We just ran across this in a slightly different scenario [Playground]:

interface test
{
    a: number;
    b: boolean;
}

const x: test = { a: 0, b: false };
const y: test = { a: 1, b: true };

// Is Okay:
const simple: keyof test = "b";
x[simple] = y[simple];    // <-- All good!

// Is Bad:
const props: Array<keyof test> = ["a", "b"];
for (const prop of props)
{
     // Type 'number | boolean' is not assignable to type 'never'.
     //   Type 'number' is not assignable to type 'never'.(2322)
    x[prop] = y[prop];    // <-- Error!
}

@fatcerberus
Copy link

@ericdrobinson Just to clear up any potential confusion, your "simple case" works because simple gets narrowed to "b" on assignment, not because there's any special case for that in the compiler.

@ericdrobinson
Copy link

ericdrobinson commented Jan 6, 2022

@fatcerberus Ahh, yes, good point. Definitely better to be clear on that for this issue.

Your pointer also made me realize that there's an even simpler example that shows the issue [Playground]:

interface test
{
    a: number;
    b: boolean;
}

const x: test = { a: 0, b: false };
const y: test = { a: 1, b: true };

declare const prop: keyof test;

// Type 'number | boolean' is not assignable to type 'never'.
//   Type 'number' is not assignable to type 'never'.(2322)
x[prop] = y[prop];    // <-- Error!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experience Enhancement Noncontroversial enhancements Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

8 participants