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

Feature: Auto inferred class properties #13068

Closed
unional opened this issue Dec 20, 2016 · 10 comments
Closed

Feature: Auto inferred class properties #13068

unional opened this issue Dec 20, 2016 · 10 comments
Labels
Duplicate An existing issue was already created

Comments

@unional
Copy link
Contributor

unional commented Dec 20, 2016

Don't know would this go against "type safety" in general.
I think with flow analysis this is now possible.

class Foo() {

  constructor() {
    this.a = 1  // automatically create property `private a: any` or `private a: number`
  }

  do() {
    this.b = this.a 
    // automatically create property `private b: any` or `private b: number | undefined`
    // because `do()` may not be called.
  }
  do2() {
    this.c = this.b 
    // automatically create proeprty `private c: any` or `private c: number | undefined`
    // because `do()` and `do2()` many not be called.
  }
}

EDIT: It's a pain to see allowJS works with Javascript like this but not within TypeScript. 🌷
EDIT2: add private
EDIT3: yay, my first "thumb-down", showing that I'm pushing the boundary. 🌷

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Dec 20, 2016

You can check out #4955 and our discussion at #5196. One of the major problems is that you could misspell a property within the constructor, or subclass constructor, and accidentally declare two different things.

In principle, tooling could help with this anyway (a quick fix for "do you want to declare new properties for this variable?").

@ahejlsberg @mhegazy @RyanCavanaugh do we feel we want to reconsider this?

@unional
Copy link
Contributor Author

unional commented Dec 20, 2016

That's a quick response. 👏

IMO the misspell is not much of a concern in practice, as nowadays engineers always use an editor/IDE with completion support (the key strong reason of writing TypeScript over JavaScript).
So the engineer will notice the misspell pretty easily.

Also, I think the compiler should not concern about misspell because I can happily class Foo { a1; number, a2: number, ... }.

And user should write tests for their logic to begin with. 🌷

@zpdDG4gta8XKpMCd
Copy link

@DanielRosenwasser

please no

since it doesn't have any explicit syntax (like private, protected, public with auto initialized properties) there is a very high chance of misspells and unwanted garbage in the code

that's the drawback

now what's the benefit of it?

@zpdDG4gta8XKpMCd
Copy link

zpdDG4gta8XKpMCd commented Dec 20, 2016

partial initialization is a nasty thing, a class should be fully initialized in its constructor, all "partial" logic should be taken outside of it

if so, then auto-properties are suffice:

class C {
 constructor(
  public a = 1,
  private b = a,
  protected c = b
 ) {}
}

let's not encourage bad patterns

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Dec 20, 2016

now what's the benefit of it?

Part of my job is to grow the TypeScript community to new users and audiences. One of the difficult things is decisions to balance some of the tradeoffs between what we consider to be ideal princples and ease of use for new users. A good example of this is making declaration files optional in TypeScript 2.1 - if you don't have noImplicitAny, then you can include a package as long as it's installed in your node_modules.

Some on the team were wary about this whether this was the right principle to take, but my view is that it was, especially given how much we'd heard from existing users about the difficulties of the existing experience. So far, I still think that this was the right decision.

So the thing that @unional mentions that is particularly difficult to justify to myself is the following:

It's a pain to see allowJS works with Javascript like this but not within TypeScript.

We have the infrastructure to make TypeScript do something like the right thing, but it's intentionally not surfaced. It seems a little strange that JavaScript users do get some better behavior, but TypeScript users don't. For example, users of Salsa (our JavaScript language service, and those of allowJs) get:

  • Support for ES5-style classes.
  • Functions with automatically contextually-typed this functions when assigning to prototype properties.
  • Correct behavior when utilizing require, module.exports, or exports.
  • JSDoc type declarations.

To make the migration experience better, we provide allowJs, but as you switch from a .js file to a .ts file, we need to make certain decisions, but with #6802, the line separating the experience between JS and TS may get blurry.

Is our stance that TypeScript mandates a class is written in a certain manner? If you can get great support in Salsa, but you can also tighten your settings, what are the tradeoffs involved? Is the idea that you must be explicit about your intent by default? That TypeScript has a more well-defined static semantics for type-checking?

For what it's worth, my own preferences are probably close to yours on this issue @Aleksey-Bykov - I kind of prefer that TypeScript pushes us towards an explicit style. But this is something that is good to discuss in general.

@DanielRosenwasser
Copy link
Member

I've opened up #13071 in favor of this issue. I think I'm going to mark this as a duplicate and we can consider revisiting the topic if editor quick-fixes are still too cumbersome to address the issue.

@DanielRosenwasser DanielRosenwasser added the Duplicate An existing issue was already created label Dec 20, 2016
@zpdDG4gta8XKpMCd
Copy link

zpdDG4gta8XKpMCd commented Dec 20, 2016

didn't know code rewrites have landed, is there a basic example in TS API?

@mhegazy
Copy link
Contributor

mhegazy commented Dec 21, 2016

didn't know code rewrites have landed, is there a basic example in TS API?

https://github.com/Microsoft/TypeScript/tree/master/src/services/codefixes

@zpdDG4gta8XKpMCd
Copy link

zpdDG4gta8XKpMCd commented Dec 21, 2016 via email

@mhegazy
Copy link
Contributor

mhegazy commented Dec 21, 2016

should be visible in the next version of VS Code and the next version of VS 2017.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants