Skip to content

Incorrect type assignment error #17561

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
Roaders opened this issue Aug 2, 2017 · 7 comments
Closed

Incorrect type assignment error #17561

Roaders opened this issue Aug 2, 2017 · 7 comments
Labels
Won't Fix The severity and priority of this issue do not warrant the time or complexity needed to fix it

Comments

@Roaders
Copy link

Roaders commented Aug 2, 2017

TypeScript Version: 2.4.0 / nightly (2.5.0-dev.201xxxxx)

Code

interface typeOne{
    name?: string;
}

interface typeTwo{
    name: string;
}

function doStuff(input: typeOne) {

    let objectWithName: typeTwo;

    if (input.name != null){
        objectWithName = input;
    }
}

(playground)
Expected behavior:
should be able to assign input to objectWithName as we have already tested to see that name is defined.

Actual behavior:
Get an error:
Type 'typeOne' is not assignable to type 'typeTwo'. Property 'name' is optional in type 'typeOne' but required in type 'typeTwo'.

@kitsonk
Copy link
Contributor

kitsonk commented Aug 2, 2017

The error is correct. It would be unsound to assign typeOne to typeTwo. Just because input.name is present at the moment, it is a reference to an object, and therefore doesn't always mean it will be assigned, therefore TypeScript is preventing you from making an unsound assignment. Where as typeTwo can be assigned to typeOne safely.

@Roaders
Copy link
Author

Roaders commented Aug 2, 2017

Ok... I can see your argument. However this still fails:

interface typeOne{
    name?: string;
}

interface typeTwo{
    name: string;
}

function doStuff(input: typeOne) {

    let objectWithName: typeTwo;

    if (input.name != null){
        objectWithName = {...input};
    }
}

@jcalz
Copy link
Contributor

jcalz commented Aug 2, 2017

@kitsonk I doubt that's what's going on. TypeScript allows unsound reassignments in many places; for example:

const t2: typeTwo = {name: 'okay'}
const t1: typeOne = t2;
t1.name = undefined; // no error
console.log(typeof t2.name) // 'undefined'

In the absence of in/out parameters or strict readonly enforcement, this kind of thing just happens.

Additionally, discriminated unions specifically allow assignments like above, despite being unsound in the same mutation cases:

interface A {
  type: 'A';
}
interface B {
  type: 'B';
}
function doMoreStuff(input: A | B) {
  let b: B;
  if (input.type == 'B') {
    b = input; // no error
  }
}

My guess is that the control-flow analysis just isn't trying to narrow the type of the object containing a property if the type of the property is narrowed. In fact you can make the compiler narrow the parent object, with a user-defined type guard:

function hasStringName(x: any): x is { name: string } {
  return (typeof x.name === 'string');
}

function doStuff(input: typeOne) {
    let objectWithName: typeTwo;
    if (hasStringName(input)) {      
        objectWithName = input;  // no error
    }
}

which would be my suggested workaround.

@RyanCavanaugh
Copy link
Member

@sandersn spread operator bug? Thoughts?

@sandersn
Copy link
Member

sandersn commented Aug 2, 2017

@RyanCavanaugh The behaviour is the same whether or not you use the spread operator. I think @Roaders was trying to show that the error happens even if you try to get the compiler to consider the properties of typeTwo separately.

I'm pretty sure this is just a feature that control flow doesn't have right now — it mostly just narrows, and there's nothing to narrow here because typeTwo isn't a union. The feature that you would like to have is for the check input.name != null to assign a new type { name: string } that is based on the declared type typeTwo = { name?: string }.

Note: You'll see that narrowing is occurring if you look at the type of input.name inside the if — the type string | undefined is narrowed to string.

@sandersn sandersn added the Suggestion An idea for TypeScript label Aug 2, 2017
@sandersn
Copy link
Member

sandersn commented Aug 3, 2017

@rbuckton and @weswigham suggested that getSpreadType or its caller might be able to call getFlowTypeOfReference on each property to get the narrowed type. This has a number of implementation obstacles (see below), but would allow at least the spread version of the repro code to work.

  1. getSpreadType operates on types only, not syntactic references, and it doesn't even distinguish between a ...spread and the normal: 'properties' next to it. The caller just packages up normal properties into anonymous object types. Perhaps I could track each type back to a declaration and if it has one, then assume it comes from a spread.
  2. Even with a reference in hand, the reference is actually the same each for each property in the spread — it's just the ...spread itself. This means that control flow will need a special case that understands that properties match references to a spread of their parent.

@RyanCavanaugh RyanCavanaugh added Won't Fix The severity and priority of this issue do not warrant the time or complexity needed to fix it and removed Suggestion An idea for TypeScript labels Aug 15, 2018
@RyanCavanaugh
Copy link
Member

Doesn't seem all that impactful, plus it's technically not sound

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Won't Fix The severity and priority of this issue do not warrant the time or complexity needed to fix it
Projects
None yet
Development

No branches or pull requests

5 participants