Skip to content

Inability to use typeof on an ES private property #47595

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

Open
rraziel opened this issue Jan 25, 2022 · 9 comments Β· Fixed by #47696
Open

Inability to use typeof on an ES private property #47595

rraziel opened this issue Jan 25, 2022 · 9 comments Β· Fixed by #47696
Labels
Experience Enhancement Noncontroversial enhancements Help Wanted You can do this Suggestion An idea for TypeScript
Milestone

Comments

@rraziel
Copy link

rraziel commented Jan 25, 2022

Bug Report

πŸ”Ž Search Terms

  • typeof
  • private

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about private and typeof

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

class Example {
  #privateProp = 'hello';

  constructor() {
    const p: typeof this.#privateProp = 'world';
    const p2: Example['#privateProp'] = 'world';
  }
}

The example is kept short for reproduction, but in practice this is more for cases where you would want to use keyof this.#someProperty and similar constructs (since here the type could just be inferred).

πŸ™ Actual behavior

Specifying a type using const name: typeof this.#anESprivateProperty is not possible.

For p, the error is that an identifier is expected (because it's a PrivateIdentifier and not an Identifier from the compiler's pov?).

For p2, the error is that the property does not exist on the type, which is logical when done from "outside" the class where the property should not be visible.

πŸ™‚ Expected behavior

Having the ability to use typeof on an ES private property from "inside" the class itself where that property is visible, e.g. like in the example code above.

@RyanCavanaugh RyanCavanaugh added Experience Enhancement Noncontroversial enhancements Help Wanted You can do this Suggestion An idea for TypeScript labels Jan 25, 2022
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Jan 25, 2022
@rraziel
Copy link
Author

rraziel commented Jan 25, 2022

I wouldn't mind giving it a shot with a PR, but just to see if I'm at least on the right track before I start going down that path:

  • NodeFactory.createTypeQueryNode (when doing typeof this.#abc) needs an entity name
  • NodeFactory.createQualifiedName can't be used for that as the second arg must be an Identifier, and here it's a PrivateIdentifier

Would this be the correct way to get the feature to work?

@a-tarasyuk
Copy link
Contributor

a-tarasyuk commented Jan 26, 2022

@RyanCavanaugh Do we need to support this syntax?

const p2: Example['#privateProp'] = 'world';

// will never use this syntax, already taken:
const badAlways: C["#bar"] = 3; // Error

class C {
    #a = 1;      // number
    ["#a"] = ""; // string

    constructor() {
        const a: C["#a"] = 1; // Should the checker use a number or a string?
    }
}

#a and ['#a'] are two different props - Playground

If we allow C["#a"], the next property should get type number. :)

this["#foo"] = 3; // Okay (type has index signature and "#foo" does not collide with private identifier #foo)

@fatcerberus
Copy link

obj['#privateProp'] accesses a different property at runtime than obj.#privateProp; the syntax in question refers to the former. That's what's meant by "already taken".

@dnalborczyk
Copy link

@RyanCavanaugh Do we need to support this syntax?

const p2: Example['#privateProp'] = 'world';

already in use with public properties.

class Example {
  #privateProp = 'hello';

  ['#publicProp']: number
}

type Prop = Example['#publicProp']

https://www.typescriptlang.org/play?#code/MYGwhgzhAECiAeYC2AHEBTaBvAsAKGmgGIUAnASwDcwAXdABVIHsVoBeaAcgAt0QQmnANz58hANqcSAVwBGIcsEYtOAXQBc0AHbSks9KXwBfUXhoBPFJmWsOCZGnSSZ8xTbVA

@fatcerberus
Copy link

@a-tarasyuk Since C["#a"] in your example already refers to the public property ("number is not assignable to string"); changing it to refer to the private property would be a pretty big breaking change.

@RyanCavanaugh
Copy link
Member

For now I would not support indexed access lookup (const p: C["#p"] or variants thereof); it's very messy. Accessing the type of a private property from outside the class is a huge code smell since it breaks under declaration emit, and should really be avoided.

typeof this.#p (from inside the class) should be the only way to get at it.

@dnalborczyk
Copy link

dnalborczyk commented Feb 1, 2022

typeof this.#p (from inside the class) should be the only way to get at it.

that would make sense and that's pretty much the same how symbols as properties are being handled.

@rbuckton
Copy link
Member

rbuckton commented May 4, 2022

I posted this on #48640, but reposting here as well:

I think a better approach would have been to allow #name in an indexed access type, rather than in a type query, i.e.:

class C {
  #x: Foo;
  private _x: Foo;

  method() {
    const p1: this[#x] = this.#x;
    const p2: this["_x"] = this._x;

    const obj = this;
    const p3: typeof obj[#x] = this.#x; // parsed as `(typeof obj)[#x]`
    const p4: typeof obj["_x"] = this._x; // parsed as `(typeof obj)["_x"]`
  }
}

This would mean adding a PrivateNameTypeNode and we could then avoid changing TypeQueryNode or QualifiedName in any way. I would also expect that #x in a type is only legal inside of the class body, just like how a runtime private name is scoped.

If you need to access the type of a private named class member outside of the class, you would need to introduce a type alias or interface for it. If a class makes a private-named index access type public, our declaration emit would also need to synthesize a type for it outside of the class. This means that the type import("foo").Bar[#baz] would only be legal inside of a class body that declares #baz.

And that's if we decide to go that far. Given @RyanCavanaugh's reticence, we might decide it's not worth the complexity it would add.

@jakebailey jakebailey reopened this May 4, 2022
@jakebailey
Copy link
Member

I've now reverted #47696 in #48959, reopening this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experience Enhancement Noncontroversial enhancements Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
7 participants