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

Class property inference from constructor initializations in JavaScript #4955

Closed
2 tasks
RyanCavanaugh opened this issue Sep 24, 2015 · 8 comments
Closed
2 tasks
Assignees
Labels
Committed The team has roadmapped this issue Suggestion An idea for TypeScript

Comments

@RyanCavanaugh
Copy link
Member

Summary

We will support inferring the shape of classes from assignments to properties of this in the constructor body. Example:

// Was an error, now OK
class Foo {
  constructor() {
    this.greeting = 'hello!';
  }
}
// Also now OK
var x = new Foo();
console.log(x.greeting);

Rules

All or none

If you declare any properties, either using an initializer or non-initialized declaration, you must declare all members this way:

class Alpha {
  constructor() {
    this.size = 42; // OK
  }
}

class Beta {
  size: number;
  constructor() {
    this.siz = 42; // Error
  }
}

class Gamma {
  size = 100;
  constructor() {
    this.siz = 42; // Error
  }
}

Types of members

The type of each property is the union of all assignment sources:

class Alpha {
  constructor() {
    this.size = 42;
  }
}
var x = new Alpha();
var y = x.size; // y: number

Syntactic forms

Only assignments of the form this.name = expr; are recognized. Alternate syntax, e.g.this['name'] = expr, or aliasing of this (e.g. var x = this; x.foo = bar) will not create properties.

Open questions:

  • Assignment statements only, or all assignment expressions? Chained initialization (this.x = this.y = 0) might be common
  • Do parameter properties count as declared properties?

Other issues mentioning this: #766, #2393, #2606.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Committed The team has roadmapped this issue labels Sep 24, 2015
@RyanCavanaugh RyanCavanaugh self-assigned this Sep 24, 2015
@RyanCavanaugh RyanCavanaugh changed the title Class shape inference from this property assignments Class property inference from constructor initializations Sep 24, 2015
@RyanCavanaugh RyanCavanaugh changed the title Class property inference from constructor initializations Class property inference from constructor initializations in JavaScript Sep 24, 2015
@jbondc
Copy link
Contributor

jbondc commented Oct 2, 2015

Very nice. Probably all this. assignment expressions should work.

Think delete deserves some thought too:

class Alpha {
    constructor() {
        this.size = 42;
        delete this.size;
    }
}

@RyanCavanaugh
Copy link
Member Author

What is the intent of that code? Does anyone actually do that? 😕 ❓

@jbondc
Copy link
Contributor

jbondc commented Oct 2, 2015

Wasn't able to find any convincing cases in JS. Seen this in PHP sometimes ~ this._tmp can be used as temporary variable:
https://github.com/bluelovers/Scophp/blob/64f114e521ff607f83812391579f4f74809c2f10/Scorpio/Sco/Array/Sorter/Stable.php

Not a huge issue but part of the question: parameter properties count as declared properties?

If someone wants a pure ES6 way to declare properties, I guess delete could be a way to make something optional.

@jbondc
Copy link
Contributor

jbondc commented Oct 2, 2015

More concrete:

class Alpha {
    constructor() {
         this.size = 42;
         this._tmp = {}
    }
}
class ImplementAlpha implements Alpha {}

Don't really want _tmp in there :/

@jkillian
Copy link

jkillian commented Oct 2, 2015

I assume the driving force behind this change is to let TS be able to mimic ES6-style JS classes? On one hand it seems like an unnecessary extra way to do things, but I like how it would let you copy over an ES6 class and be able to start using it with type-checking in TS.

I don't think there's a good answer for parameter properties. If they're considered declared properties, it disallows their use as shorthand in conjunction with the new this syntax. If they're not considered declared, it forces classes that don't want to use the new this syntax to have at least one non-parameter property field. I think the former of these two options is less harmful, so I'd vote to consider them declared properties (plus this is more backwards-friendly).

@RyanCavanaugh
Copy link
Member Author

Done in JavaScript branch

@nycdotnet
Copy link

@RyanCavanaugh It sounds like this is only going to be in Salsa for now based on the design meeting notes - can you confirm that? Thanks.

@RyanCavanaugh
Copy link
Member Author

@nycdotnet

It sounds like this is only going to be in Salsa for now

Correct

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Committed The team has roadmapped this issue Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants