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 does not work with private class fields #39960

Closed
lazytype opened this issue Aug 8, 2020 · 6 comments · Fixed by #39978
Closed

Class property inference from constructor does not work with private class fields #39960

lazytype opened this issue Aug 8, 2020 · 6 comments · Fixed by #39978
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue

Comments

@lazytype
Copy link

lazytype commented Aug 8, 2020

TypeScript Version: 4.0.0-dev.20200803

Search Terms: class property inference, private class fields

Expected behavior: private class field's type is inferred from its assignment in the constructor

Actual behavior: typechecker reports implicit any for private class field

Related Issues:

Code

class Example {
  #test;

  constructor(test: number) {
    this.#test = test;
  }

  get test() {
    return this.#test
  }
}
Output
"use strict";
var __classPrivateFieldSet = (this && this.__classPrivateFieldSet) || function (receiver, privateMap, value) {
    if (!privateMap.has(receiver)) {
        throw new TypeError("attempted to set private field on non-instance");
    }
    privateMap.set(receiver, value);
    return value;
};
var __classPrivateFieldGet = (this && this.__classPrivateFieldGet) || function (receiver, privateMap) {
    if (!privateMap.has(receiver)) {
        throw new TypeError("attempted to get private field on non-instance");
    }
    return privateMap.get(receiver);
};
var _test;
class Example {
    constructor(test) {
        _test.set(this, void 0);
        __classPrivateFieldSet(this, _test, test);
    }
    get test() {
        return __classPrivateFieldGet(this, _test);
    }
}
_test = new WeakMap();
Compiler Options
{
  "compilerOptions": {
    "noImplicitAny": true,
    "strictNullChecks": true,
    "strictFunctionTypes": true,
    "strictPropertyInitialization": true,
    "strictBindCallApply": true,
    "noImplicitThis": true,
    "noImplicitReturns": true,
    "alwaysStrict": true,
    "esModuleInterop": true,
    "declaration": true,
    "experimentalDecorators": true,
    "emitDecoratorMetadata": true,
    "moduleResolution": 2,
    "target": "ES2017",
    "jsx": "React",
    "module": "ESNext"
  }
}

Playground Link: Provided

@orta
Copy link
Contributor

orta commented Aug 10, 2020

I'm not sure if TS can support this, the syntax sugar for adding the type in the constructor generates a public field but the #syntax isn't allowed in the constructors params

Screen Shot 2020-08-10 at 8 28 13 AM

and

Screen Shot 2020-08-10 at 8 29 20 AM

If we added support for this we might end up taking syntax space from JavaScript

@lazytype
Copy link
Author

lazytype commented Aug 10, 2020

@orta That's not what I was referring to. I was referring to this recently released behavior which currently behaves correctly for non-private fields.
https://devblogs.microsoft.com/typescript/announcing-typescript-4-0-rc/#class-property-inference

The playground that is linked demonstrates the bug without need for modification.

@orta
Copy link
Contributor

orta commented Aug 10, 2020

Yep, I know what you're talking about 👍🏻

My point was that that syntax space is already taken for the implicit field which is created how it currently works. We'd have to add support for putting #fieldname in a constructor in order to disambiguate,

@lazytype
Copy link
Author

I'm afraid I don't really understand what you mean. The playground I linked doesn't use any TypeScript-specific syntactic sugar. Are you referring to parameter properties?
https://www.typescriptlang.org/docs/handbook/classes.html#parameter-properties

// no implicit field created
constructor(test: number) { }

// implicit field created
constructor(public test: number) { }

Except for the one type annotation, the playground I linked is valid vanilla JS so I don't really understand what that has to do with what is or isn't valid in the constructor's params.

Here's another demonstration that's maybe more clear.

class Example {
  #test;  // should not be implicit any error

  constructor() {
    this.#test = 42;
  }
}

@DanielRosenwasser DanielRosenwasser added the Bug A bug in TypeScript label Aug 10, 2020
@DanielRosenwasser
Copy link
Member

This works with no errors in noImplicitAny

class Example {
  test;

  constructor(test: number) {
    this.test = test;
  }

  get publicTest() {
    return this.test
  }
}

But the original example code doesn't. That should be fixed.

@DanielRosenwasser
Copy link
Member

@ahejlsberg think this would be trivial to fix before the 4.0 stable?

@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Aug 10, 2020
@RyanCavanaugh RyanCavanaugh added Rescheduled This issue was previously scheduled to an earlier milestone and removed Rescheduled This issue was previously scheduled to an earlier milestone labels Aug 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants