Suggestion: allow get/set accessors to be of different types

[irakliy81]

It would be great if there was a way to relax the current constraint of requiring get/set accessors to have the same type. this would be helpful in a situation like this:

class MyClass {

    private _myDate: moment.Moment;

    get myDate(): moment.Moment {
        return this._myDate;
    }

    set myDate(value: Date | moment.Moment) {
        this._myDate = moment(value);
    }
}

Currently, this does not seems to be possible, and I have to resort to something like this:

class MyClass {

    private _myDate: moment.Moment;

    get myDate(): moment.Moment {
        return this._myDate;
    }

    set myDate(value: moment.Moment) {
        assert.fail('Setter for myDate is not available. Please use: setMyDate() instead');
    }

    setMyDate(value: Date | moment.Moment) {
        this._myDate = moment(value);
    }
}

This is far from ideal, and the code would be much cleaner if different types would be allowed.

Thanks!

[danquirk]

I can see how this would be nice here (and this has been requested before although I can't find the issue now) but it's not possible whether or not the utility is enough to warrant it. The problem is that accessors are not surfaced in .d.ts any differently than normal properties since they appear the same from that perspective. Meaning there's no differentiation between the getter and setter so there's no way to a) require an implementation use an accessor rather than a single instance member and b) specify the difference in types between the getter and setter.

[irakliy81]

Thank you for the quick reply, Dan. I'll follow the less elegant way. Thanks for the great work!

[tkrotoff]

A JavaScript getter and setter with different types is perfectly valid and works great and I believe it is this feature's main advantage/purpose. Having to provide a setMyDate() just to please TypeScript ruins it.

Think also about pure JS libraries that will follow this pattern: the .d.ts will have to expose an union or any.

The problem is that accessors are not surfaced in .d.ts any differently than normal properties

Then this limitation should be fixed and this issue should stay open:

// MyClass.d.ts

// Instead of generating:
declare class MyClass {
  myDate: moment.Moment;
}

// Should generate:
declare class MyClass {
  get myDate(): moment.Moment;
  set myDate(value: Date | moment.Moment);
}

// Or shorter syntax:
declare class MyClass {
  myDate: (get: moment.Moment, set: Date | moment.Moment);
  // and 'fooBar: string' being a shorthand for 'fooBar: (get: string, set: string)'
}

[RyanCavanaugh]

I realize this is just an opinion, but writing a setter such that a.x === y is not true immediately after a.x = y; is a giant red flag. How do consumers of a library like that know which properties have magic side effects and which don't?

[tkrotoff]

How do [you] know which properties have magic side effects and which don't?

In JavaScript it can be un-intuitive, in TypeScript the tools (IDE + compiler) will complain. Why do we love TypeScript again? :)

[kitsonk]

A JavaScript getter and setter with different types is perfectly valid and works great and I believe it is this feature's main advantage/purpose.

This is arguing that JavaScript is weakly typed, so TypeScript should be weakly typed. :-S

[tkrotoff]

This is arguing that JavaScript is weakly typed, so TypeScript should be weakly typed

C# allows it and that does not make this language weakly typed C# does not allow get/set of different types.

[kitsonk]

C# allows it and that does not make this language weakly typed C# does not allow get/set of different types.

😉

[paulwalker]

No one is arguing (as far as I can see) that accessors should be weakly typed, they are arguing that we should have the flexibility to define the type(s).

Often it is needed to copy a plain old object to object instances.

    get fields(): Field[] {
      return this._fields;
    }

    set fields(value: any[]) {
      this._fields = value.map(Field.fromJson);
    }

That is much nicer than the alternative and allows my setter to encapsulate the exact type of logic setters are made for.

[danquirk]

@paulwalker the pattern you use there (a setter taking an any and a getter returning a more specific type) is valid today.

[paulwalker]

@danquirk Great to know, thank you! It looks like I just needed to update my IDE plugin compiler for ST.

[sccolbert]

@danquirk That doesn't seem to work according to the playground (or version 1.6.2):
http://www.typescriptlang.org/Playground#src=%0A%0Aclass%20Foo%20%7B%0A%0A%20%20get%20items()%3A%20string%5B%5D%20%7B%0A%09%20%20return%20%5B%5D%3B%0A%20%20%7D%0A%20%20%0A%20%20set%20items(value%3A%20any)%20%7B%0A%09%20%20%0A%20%20%7D%0A%7D

[jasongrout]

I just tested with typescript@next (Version 1.8.0-dev.20151102), and also have an error.

~$ tsc --version
message TS6029: Version 1.8.0-dev.20151102
~$ cat a.ts
class A {
    get something(): number {return 5;}
    set something(x: any) {}
}

~$ tsc -t es5 a.ts
a.ts(2,2): error TS2380: 'get' and 'set' accessor must have the same type.
a.ts(3,2): error TS2380: 'get' and 'set' accessor must have the same type.

[paulwalker]

Ironically, after updating my Sublime linter, it no longer threw an error, which is using TypeScript 1.7.x. I've been assuming it is a forthcoming enhancement in 1.7+, so perhaps 1.8.0 regressed.

[christianacca]

Even with the version of visual studio code (0.10.5 (December 2015)) that supports typescript 1.7.5 and with typescript 1.7.5 installed globally on my machine this is still a problem:

So what version this will be supported?
Thanks

[RyanCavanaugh]

I think Dan was mistaken. The getter and the setter must be of identical type.

[christianacca]

shame. would have been a nice feature for when writing page objects for use in protractor tests.

I would have been able to write a protractor test:

po.email = "[email protected]";
expect(po.email).toBe("[email protected]");

... by authoring a page object:

class PageObject {
    get email(): webdriver.promise.Promise<string> {
        return element(by.model("ctrl.user.email")).getAttribute("value")
    }
    set email(value: any) {
        element(by.model("ctrl.user.email")).clear().sendKeys(value);
    }
}

[RyanCavanaugh]

What about that code requires the setter to be of type any ?

[christianacca]

The getter will return a webdriver.promise.Promise<string> yet the setter I want to assign a string value.

Maybe the following longer form of the protractor test makes it clearer:

po.email = "[email protected]";
var currentEmail : webdriver.promise.Promise<string> = po.email;
expect(currentEmail).toBe("[email protected]")

[Arnavion]

@RyanCavanaugh With the introduction of null annotations, this prevents code that allows calling a setter with null to set it to some default value.

class Style {
    private _width: number = 5;

    // `: number | null` encumbers callers with unnecessary `!`
    get width(): number {
        return this._width;
    }

    // `: number` prevents callers from passing in null
    set width(newWidth: number | null) {
        if (newWidth === null) {
            this._width = 5;
        }
        else {
            this._width = newWidth;
        }
    }
}

Could you consider atleast allowing the types to differ in the presence of | null and | undefined ?

[MaklaCof]

This would really be a nice feature.

[artyil]

So will this be a feature?

[kitsonk]

@artyil it is closed and tagged By Design which indicates that currently there is not any plans to add it. If you have a compelling use case which you think overrides the concerns expressed above, you can feel free to make your case and additional feedback may make the core team reconsider their position.

[paulwalker]

@kitsonk I think more than enough compelling use cases have been provided in the above comments. While the current design follows a common pattern of most other typed languages with this restriction, it is unnecessary and overly restrictive in the context of Javascript. While it is by design, the design is wrong.

[MaklaCof]

I agree.

[somebody1234]

Honestly, I really try to refrain from being prescriptive here about how to write code, but I truly object to the idea that this code

foo.bar = "hello";
console.log(foo.bar);

should ever print anything than "hello" in a language that attempts to have sane semantics. Properties should have behavior that is indistinguishable from fields to external observers.

While this may be a valid point, in reality this is not the case.
Or more like, sure, it shouldn't, but there's a difference between "we don't recommend this but sure" and "we don't recommend this so just don't do it"

[ricmoo]

This is a common pattern in DOM though, no? foo.innerText = 5 and then a console.log(foo.innerText)? It allows an object to do its own coercion on its properties. I want to be able to assign an object to a req.headers, and have it converted to a Headers object.

[somebody1234]

Indeed, this (dom setters converting types) been mentioned above multiple times I believe

[somebody1234]

@shicks re: narrowing...
I feel like it should definitely only narrow if it's a subtype.
If it's merely overlapping, I feel like you can't really be sure that the get is returning one of those types (since it'd probably rely on an earlier set to enter a certain type of value?)
You'd be able to use as anyway, not an ideal solution but without examples I'm not sure it's good default behavior

[RyanCavanaugh]

There's a Playground build for PR 42425 available for trying out and I'd appreciate any feedback on how it meets folks' needs here. Thanks! See comments in #42425 on how this works.

[thw0rted]

Ryan, is the team planning to refine the DOM defs (in lib.dom.d.ts) if your PR goes through? Is that even possible using the current build process? It'd be a breaking change, but only for code that isn't technically correct (the best kind of correct).

[RyanCavanaugh]

We'll take feedback on the DOM if this goes in. I think we'd still be keeping a pretty tight bar on what's allowed there; most of the DOM has coercing setters where the coercion is in the vein of similarly-disallowed coercions like "5" * "11". More "obvious" coercions like string -> URL seem like the kind of thing we'd be open to.

[hopeless-programmer-online]

Any updates?

In the following code example:

class vec2 {
  x : number
  y : number

  dot(other : vec2) : number
}
class vec3 {
  get xy() : vec2
  set xy(xy : { x : number, y : number })
}

we are forced to return only { x : number, y : number } on get x() or to accept only vec2 on set x().
While the ideal approach would be to get fully operable vec2 with all its methods and to set anything that has x and y.

[thw0rted]

In #42425, Ryan said "First, the type of the getter must be assignable to the type of the setter. In other words, the assignment obj.x = obj.x; must be legal assuming obj.x is writable at all.... These prevent novel unsoundness from occurring and makes this feature unimpactful from a type relational perspective, which limits its risk." It sounds like the incoming feature will not support your use case.

That said, look immediately above your post: #43662 was just opened, which points out that "[Element#style] is one where you can’t just make the set type string | CSSStyleDeclaration because assigning a CSSStyleDeclaration back to the accessor doesn’t work". Maybe follow / upvote that issue and watch for more discussion there?

[pckhoi]

There is no reason to think that separately typed getter/setter should not be used in a greenfield project. JavaScript isn't statically typed and it has its own benefits: more terse and expressive syntax. Not having to remember 200 different method names that do a single thing with slightly different input is a great thing! This is even more powerful when taking into account new ES features such as proxy. This is just very valuable to library author.

While we're at it how about adding differently typed getter/setter to interface also? E.g.

interface SeriesGetter {
  get [prop:string]: Series
  set [prop:string]: number | string | null | undefined | Series
}

Only reason why I want this on interface is because this is the only way for proxy to work in typescript.

[shicks]

In the following code example:

class vec2 {
  x : number
  y : number

  dot(other : vec2) : number
}
class vec3 {
  get xy() : vec2
  set xy(xy : { x : number, y : number })
}

we are forced to return only { x : number, y : number } on get x() or to accept only vec2 on set x().
While the ideal approach would be to get fully operable vec2 with all its methods and to set anything that has x and y.

This already works in the proposed PR. See this example.

While we're at it how about adding differently typed getter/setter to interface also? E.g.

interface SeriesGetter {
  get [prop:string]: Series
  set [prop:string]: number | string | null | undefined | Series
}

Only reason why I want this on interface is because this is the only way for proxy to work in typescript.

To be pedantic, it does work on interfaces (see example). What doesn't work is index signatures, which honestly I'm okay with, since it's impossible to write such a getter/setter pair without proxies, and by the time you're talking about proxies, static typing is a bit of a stretch.

[pckhoi]

Hey there are value in slapping some typing info onto proxies. Why do we need to be extreme and say either proxy or static typing?

[mike-marcacci]

I am extremely interested in this because it is a prerequisite for my proposal to fix the major soundness issue with index signatures in a way that has a fairly well-considered migration path (if I can say so myself).

Of course, tracking get/set separately does raise some new questions, especially around type refinement. For example:

type SomeMap = {
  get [key: string](): string | undefined;
  set [key: string](string): string;
}

function(map: SomeMap) {
  map.alpha = "beta";
  const { alpha } = map;
  // Here I would expect alpha to be refined to "beta" because
  // we are treating the object as a "map". This follows current
  // refinement rules, and is what I'm interested in.
}

type Foo =  {
  get alpha(): string | number,
  set alpha(value: string | boolean): string | boolean
}

function(foo: Foo) {
  foo.alpha = true;
  const { alpha } = foo;
  // What value is foo? We can't really refine our type here...
}

While the former scenario is almost certainly the biggest use-case here (and the one I care about), keeping the desired logic breaks the latter; supporting the second case severely limits usefulness of the former without some additional mechanism for describing when these the compiler can make these refinements.

I think perhaps some additional mechanism for this needs to be drawn up.

[thw0rted]

Mike, you should make a playground example to illustrate what you think is going on. You say that you'd expect const {alpha} to be typed as "beta" (literal) following current refinement rules, but it actually narrows to string.

From that same Playground, even without the new feature you wind up with unsound / incorrect assumptions about property accessors. This is of course dumb:

class Bar {
  private _x: string | number = 1;
  get x(): string | number {
    return this._x;
  }
  set x(val: string | number) {
    if (Number.isInteger(val)) {
      this._x = `${val}`;
    } else {
      this._x = Number(val);
    }
  }
}

const b = new Bar();

b.x = 5
const x1 = b.x; // number
console.log(`number? no, ${typeof x1}`); // logs "string"

b.x = "3";
const x2 = b.x; // string
console.log(`string? no, ${typeof x2}`); // logs "number"

but I'm sure there are more reasonable real-world use cases.

[pckhoi]

I would be happy enough if type refinement simply follow to the letters of what the declaration describe for now. map.alpha can just be string | undefined, foo.alpha can just be string | number.

Maybe extra fine type refinement can be declared somehow in the future but I think it's out of this issue's scope.

[SrBrahma]

@RyanCavanaugh Close this? #42425

[RyanCavanaugh]

Yep. Folks can track #43662 for possible loosening of the get/set relation.