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

3.3.1 regression in type inference #29743

Open
eamodio opened this issue Feb 5, 2019 · 8 comments
Open

3.3.1 regression in type inference #29743

eamodio opened this issue Feb 5, 2019 · 8 comments
Labels
Bug A bug in TypeScript
Milestone

Comments

@eamodio
Copy link

eamodio commented Feb 5, 2019

TypeScript Version: 3.3.1 & 3.4.0-dev.20190202

Search Terms:

Code

class Foo {
    foo: number = 0;
}

class FooBar extends Foo {
    bar: number = 10;
}

async function foo(f: Foo | number) {
    if (f instanceof FooBar) {
        try {
            f = await (async (): Promise<Foo | number> => {
                throw new Error();
            })();
        }
        catch (ex) {
            console.log(f.bar); // 3.3.1 has an error on this line -- "Property 'bar' does not exist on type 'number | Foo'. Property 'bar' does not exist on type 'number'"
        }
    }
}

foo(new FooBar());

Expected behavior:

No error as was in TypeScript 3.2.4

Actual behavior:

Error Property 'bar' does not exist on type 'number | Foo'. Property 'bar' does not exist on type 'number'

Playground Link:
http://www.typescriptlang.org/play/index.html#src=class%20Foo%20%7B%0D%0A%20%20%20%20foo%3A%20number%20%3D%200%3B%0D%0A%7D%0D%0A%0D%0Aclass%20FooBar%20extends%20Foo%20%7B%0D%0A%20%20%20%20bar%3A%20number%20%3D%2010%3B%0D%0A%7D%0D%0A%0D%0Aasync%20function%20foo(f%3A%20Foo%20%7C%20number)%20%7B%0D%0A%20%20%20%20if%20(f%20instanceof%20FooBar)%20%7B%0D%0A%20%20%20%20%20%20%20%20try%20%7B%0D%0A%20%20%20%20%20%20%20%20%20%20%20%20f%20%3D%20await%20(async%20()%3A%20Promise%3CFoo%20%7C%20number%3E%20%3D%3E%20%7B%0D%0A%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20throw%20new%20Error()%3B%0D%0A%20%20%20%20%20%20%20%20%20%20%20%20%7D)()%3B%0D%0A%20%20%20%20%20%20%20%20%7D%0D%0A%20%20%20%20%20%20%20%20catch%20(ex)%20%7B%0D%0A%20%20%20%20%20%20%20%20%20%20%20%20console.log(f.bar)%3B%0D%0A%20%20%20%20%20%20%20%20%7D%0D%0A%20%20%20%20%7D%0D%0A%7D%0D%0A%0D%0Afoo(new%20FooBar())%3B

Related Issues:

@eamodio
Copy link
Author

eamodio commented Feb 5, 2019

Here is another case

Code

function foo(o: { item?: { message?: string } }) {
    if (o.item === undefined || o.item.message === undefined) {
        o.item = (() => ({ message: 'hello' }))();
    }

    console.log(o.item.message.length);
}

Expected behavior:

No error as was in TypeScript 3.2.4

Actual behavior:

Error Object is possibly 'undefined'

Playground Link:
http://www.typescriptlang.org/play/index.html#src=function%20foo(o%3A%20%7B%20item%3F%3A%20%7B%20message%3F%3A%20string%20%7D%20%7D)%20%7B%0D%0A%20%20%20%20if%20(o.item%20%3D%3D%3D%20undefined%20%7C%7C%20o.item.message%20%3D%3D%3D%20undefined)%20%7B%0D%0A%20%20%20%20%20%20%20%20o.item%20%3D%20(()%20%3D%3E%20(%7B%20message%3A%20'hello'%20%7D))()%3B%0D%0A%20%20%20%20%7D%0D%0A%0D%0A%20%20%20%20console.log(o.item.message.length)%3B%0D%0A%7D

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Feb 5, 2019
@RyanCavanaugh
Copy link
Member

See #29513 for instanceof issues

@eamodio
Copy link
Author

eamodio commented Feb 5, 2019

@RyanCavanaugh that issue says it affects 3.2+, but I only see the issue with 3.3+, prior to 3.3 those cases worked fine for me.

@ahejlsberg
Copy link
Member

This is not related to #29513. I believe it is caused by #29466.

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript and removed Duplicate An existing issue was already created labels Feb 5, 2019
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Mar 14, 2019
@ajsharp
Copy link

ajsharp commented Mar 15, 2019

Any movement on this? I came across this today and it bit me in production. Had tests around the affected code but test suite was running via ts-node, so it didn't catch this.

@ajsharp
Copy link

ajsharp commented Mar 15, 2019

To add some detail on my particular issue (happy to open a separate issue if unrelated), this issue seems related to importing certain kinds of interfaces, possibly related to #13965. At least in my case, classes are fine. Specifically, the interface doesn't get brought into the imported javascript, but the class does.

I put together a little repo that illustrates the bug: https://github.com/ajsharp/typescript-interface-bug.

Below is the relevant part of the code. Document is an exported interface from mongoose, and Query is an exported class. Here's the type definition for Document.

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/e51e5044b8cf1bb317f5ca052cba4531d051a234/types/mongoose/index.d.ts#L3301

import { model, Document, Model, Schema, Query } from "mongoose";

export interface PersonDocument extends Document {
  name: string;
}

export interface PersonModel extends Model<PersonDocument> {}

const personSchema = new Schema({
  name: String
})

personSchema.methods.test = function() {
  // will throw a 'ReferenceError: Document is not defined' error
  if (this instanceof Document) {
    return `I'm a document!`;
  } else if (this instanceof Query) {
    return `I'm a query!`
  } else {
    return `I'm neither`
  }
}

export const Person: PersonModel = model<PersonDocument, PersonModel>('Person', personSchema);

Which generates the following javascript, which does not properly import the Document interface.

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
const mongoose_1 = require("mongoose");
const personSchema = new mongoose_1.Schema({
    name: String
});
personSchema.methods.test = function () {
    // will cause a Document undefined error
    if (this instanceof Document) {
        return `I'm a document!`;
    }
    else if (this instanceof mongoose_1.Query) {
        return `I'm a query!`;
    }
    else {
        return `I'm neither`;
    }
};
exports.Person = mongoose_1.model('Person', personSchema);
//# sourceMappingURL=Person.js.map

@ajsharp
Copy link

ajsharp commented Mar 19, 2019

ping @RyanCavanaugh @ahejlsberg

@ajsharp
Copy link

ajsharp commented Mar 20, 2019

Following up here after some conversation in the typescript gitter. I was reminded that interfaces do not show up in compiled code. However, the compiler should be raising a 'Document' only refers to a type, but is being used as a value here error due to how I'm trying to use it, but it is not.

This behavior seems limited to imported modules, not local code. If I define an interface in my project and try to use it on the right side of an instanceof statement, the typescript compiler raises the proper error. But when importing an interface from an external module, it does not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants