Interface Fields/Class Properties Mapping Bug

[iam1me]

The following code demonstrates a bug between interface fields and the class properties that the TypeScript compiler accepts in their place. The IDisplayInfo interface below has a DisplayName field. The test1 class implements this interface via a setter property function. The test2 class implements this interface via a getter property function. An instance of each is instantiated and each instance gets and sets the DisplayName property. This code successfully compiles, though it will obviously error at run-time.

module myModule{
    export interface IDisplayInfo{
        DisplayName: string;
        }

    class test1 implements IDisplayInfo
    {
        private _displayName: string;

        set DisplayName(name: string) {
            this._displayName = name;
        }
    }

    class test2 implements IDisplayInfo{
        private _displayName: string;

        get DisplayName() {
            return this._displayName;
        }
    }

    var t1: test1 = new test1();
    var t2: test2 = new test2();

    t1.DisplayName = "hi";
    var dn = t1.DisplayName;

    t2.DisplayName = "goodbye";
    dn = t2.DisplayName;    
}

[RyanCavanaugh]

https://github.com/Microsoft/TypeScript/wiki/FAQ#why-are-getters-without-setters-not-considered-read-only

[iam1me]

Thanks Ryan. I did happen upon this while investigating support for read-only properties, but the intent in the example isn't to make the field read-only. Rather, the interface specifies a field that I should be able to both read and write. Yet the classes that implement this interface are not being required to support both read and write.

Note that if I left out both a getter and a setter function for DisplayName then the compiler would error.

Interestingly, I removed the interface altogether to get the following code, which also compiles, but would also clearly fail at run-time:

module myModule {
    class test1  {
        private _displayName: string;

        set DisplayName(name: string) {
            this._displayName = name;
        }
    }

    class test2  {
        private _displayName: string;

        get DisplayName() {
            return this._displayName;
        }
    }

    var t1: test1 = new test1();
    var t2: test2 = new test2();

    t1.DisplayName = "hi";
    var dn = t1.DisplayName;

    t2.DisplayName = "goodbye";
    dn = t2.DisplayName;
}

The compiler should detect the lack of a getter/setter property function respectively and not let the above work.

[mhegazy]

With TS 2.0 (now available in the nighlty drops typescript@next), your getter-only properties will be flagged as readonly. see #6532 for more details.

[iam1me]

@mhegazy - as I noted above, I'm not trying to make it readonly. The bug is that the class should be made to implement both a getter and setter, as per the interface IDisplayInfo, not only one of them.

[RyanCavanaugh]

That's the same thing. If you try to implement a read/write interface property with a read-only property, you'll see an error.

[iam1me]

I checked and I have version 1.8.2.0; I'll need to update that so that I have 2.0 (I only downloaded it a few days ago, on Friday, so I figured it was up to date. Guess not)

But that's still only half the problem, unless I am missing something. The other half is using a setter-only. You could still implement the above interface like so:

    class test1 implements IDisplayInfo
    {
        private _displayName: string;

        set DisplayName(name: string) {
            this._displayName = name;
        }
    }

Or does 2.0 catch this as well now? If so that will simplify things for me; I was starting to write up a more detailed API for defining and interacting with properties.

[iam1me]

OK, I went and installed the latest dev version (2.0) via npm. I confirmed that the following typescript code is "successfully" compiled:

module myModule {
    export interface IDisplayInfo {
        DisplayName: string;
    }

    export class test1 implements IDisplayInfo  {
        private _displayName: string;

        set DisplayName(name: string) {
            this._displayName = name;
        }
    }

    var t1: test1 = new test1();

    t1.DisplayName = "hi";
    var dn = t1.DisplayName; /// ERROR: Interface allows this, but class doesn't implement it
}

This is obviously a bug since the TypeScript compiler is not requiring the non-abstract class test1 to fully implement the interface IDisplayInfo. If you try to request the DisplayName value from an instance of test1, it will return undefined

I did confirm, however, that only supplying a getter property function for DisplayName will not compile, as expected from the above conversation.

[RyanCavanaugh]

We're not really going to do a bunch of extra work to handle setter-only properties. The "mistake" of only writing a setter is sort of inconceivable, and it is legal (i.e. is not a runtime error) to not have a getter (it just returns undefined, which could be what you intended).

[iam1me]

That seems kind of silly to not support such a basic case. Isn't the primary benefit of TypeScript compile time type safety? When I compile a bunch of classes that use an interface and it succeeds, I'm trusting that TypeScript successfully verified that those classes do in fact implement that interface and that I can safely use them accordingly. While I agree this is not as common as accidently providing a getter only - this is certainly an issue that will cause run-time bugs - bugs that could have been prevented via proper compile time validation.

If you all think that it is of no importance for TypeScript to provide such basic compile time type-safety then one must question the entire reason for using it.

[mhegazy]

you can find some background on why the we chose this design approach in #6614.

Readonly support is a new feature (see #6614). The support will be available in the next release of TypeScript (TypeScript 2.0)[https://github.com/Microsoft/TypeScript/wiki/Roadmap#20]; before that there was no notion of read-only vs read/write properties.

Introducing a new concept like readonly, we had two choices; 1. all un-annotated properties are read/write, 2. all un-annoteated properties are "unknown". From a purity point of view, 1 is the best choice, but in practice, it will cause a lot of pain to users as now the same interface/class they had before mean different thing. Also note that users in the past had no way of declaring intent. so your property bag interface, that would have been write using read only looks like your public state API, and the compiler has no way of telling which was meant to readonly and which is not. We ended up landing on 2. In this design only readonly-annotated properties are considered readonly, and un-annotated are not checked; thus, you can assign (or in this case implement) an un-annotated interface properties with readonly properties, but you can not access these properties directly. This allows for catching a lot of the errors without having to introduce breaking changes that would cause ripples of pain throughout the community.