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

Typescript 3.8.3 : Sharp notation for private method must be alloweded #37677

Closed
5 tasks done
Binau opened this issue Mar 29, 2020 · 35 comments
Closed
5 tasks done

Typescript 3.8.3 : Sharp notation for private method must be alloweded #37677

Binau opened this issue Mar 29, 2020 · 35 comments
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@Binau
Copy link

Binau commented Mar 29, 2020

Search Terms

TS18022, sharp

Suggestion

Sharp notation for private method must be alloweded : https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/Class_fields#Private_instance_methods

Use Cases

class Test {
#test() {}
}

Examples

class Test {
#test() {}
}
=> TS18022: A method cannot be named with a private identifier.

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@orta orta added the Question An issue which isn't directly actionable in code label Mar 29, 2020
@orta
Copy link
Contributor

orta commented Mar 29, 2020

It is supported: https://www.typescriptlang.org/play/?ts=3.8.0-beta&e=70#example/private-class-fields

@Binau
Copy link
Author

Binau commented Mar 29, 2020

i think their is not private method in your example. try to write #greet() to replace greet() method in your example, and you have this error message : "A method cannot be named with a private identifier.(18022)".

@orta orta removed the Question An issue which isn't directly actionable in code label Mar 29, 2020
@orta
Copy link
Contributor

orta commented Mar 29, 2020

True yeah, and I can't seem to find another issue about this 👍

@Jamesernator
Copy link

The relevant proposal for private methods is here: https://github.com/tc39/proposal-private-methods

Private methods are only shipping in XS so far, but they are in active development in V8.

@RyanCavanaugh RyanCavanaugh added In Discussion Not yet reached consensus Suggestion An idea for TypeScript labels Mar 30, 2020
@Binau
Copy link
Author

Binau commented Apr 4, 2020

In the same way, static private attribute in class must be allowed too.
however :

class Test {
static #TEST:any;
}

do :

TS18019: 'static' modifier cannot be used with a private identifier

Référence : https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/Class_fields#Private_fields
proposal : https://github.com/tc39/proposal-static-class-features
Already compatible in edge (79), node (13), chrome

@davidbayo10
Copy link

In the same way, static private attribute in class must be allowed too.
however :

class Test {
static #TEST:any;
}

do :

TS18019: 'static' modifier cannot be used with a private identifier

Référence : https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/Class_fields#Private_fields
proposal : https://github.com/tc39/proposal-static-class-features
Already compatible in edge (79), node (13), chrome

This is a problem...

class S {

  static #instance: S;

  static getInstance(): S {
    if (!this.#instance) {
      this.#instance = new S();
    }

    return this.#instance;
  }

}

How can I perform this?

Thank you a lot in advance

@mellonis
Copy link

In the same way, static private attribute in class must be allowed too.
however :

class Test {
static #TEST:any;
}

do :

TS18019: 'static' modifier cannot be used with a private identifier

Référence : https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/Class_fields#Private_fields
proposal : https://github.com/tc39/proposal-static-class-features
Already compatible in edge (79), node (13), chrome

This is a problem...

class S {

  static #instance: S;

  static getInstance(): S {
    if (!this.#instance) {
      this.#instance = new S();
    }

    return this.#instance;
  }

}

How can I perform this?

Thank you a lot in advance

Private properties and methods should not be accessible from outside the class instance.
Own methods of a class should have access to private methods of a class if they are static or not.

Can be a constructor private? If it's not, is this singleton example valid?

@davidbayo10
Copy link

#instance is a private and static field inside S class and getInstance is static too. As you can create this kind of static fields and methods in NodeJS, it should work in TS

https://node.green/#ESNEXT-candidate--stage-3--static-class-fields-private-static-class-fields

Node allow this way for creating singleton patterns. Try out yourself. I don't understand why TS is showing that error.

@mellonis
Copy link

It was a misunderstanding. I thought that your problem was the 'inaccessibility' of the private field #instance from the not private method getInstance(). I agree that there should be no problem.

Anyway. That's the point to use this pattern? Anyone can instantiate this class with new operator. How can I prevent this or it should be a convention in a team not to use new operator with this class?

@davidbayo10
Copy link

That's a good point. I think we can use a private constructor. I think It fits well with this pattern. But, in a singleton pattern everyone can use getInstance method or new. If getInstance method exists, then should be used

@mellonis
Copy link

I'm also unable to use private getters and setters in a class in TS.

class GSExample {
    #a: number = 0;

    get a() { // this is file
        return this.#a;
    }

    set a(value: number) { // this is file
        this.#a = value;
    }

    get #b() { // Error: An accessor cannot be named with a private identifier.(18023)
        return this.a * 2;
    }

    set #b(value: number) { // Error: An accessor cannot be named with a private identifier.(18023)
        this.a = value / 2;
    }
}

@davidbayo10
Copy link

davidbayo10 commented Apr 22, 2020

Is this possible in NodeJS? You can try in a Node console

@mellonis
Copy link

The constructor method is also a method (on my opinion) and now it cannot be private:

class CExample {
    #a: number = 0;

    #constructor(a: number) { // Error: A method cannot be named with a private identifier.(18022)
        this.#a = a;
    }
}

@mellonis
Copy link

Is this possible in NodeJS? You can try in a Node console

Not yet, unfortunately. proposal Staged 3

I have code pieces that use this feature (private getters and setters). They are transpiled by the Babel. Now I cannot rewrite them into TS. :(

@davidbayo10
Copy link

Is this possible in NodeJS? You can try in a Node console

Not yet, unfortunately. proposal Staged 3

I have code pieces that use this feature (private getters and setters). They are transpiled by the Babel. Now I cannot rewrite them into TS. :(

Then, If fails in TS is correct

@mellonis
Copy link

I mean private instance methods, private static methods, private accessor properties and private static accessor properties --- all this features are similar to each other and should be supported all together.

@EnzoAlbornoz
Copy link

EnzoAlbornoz commented May 13, 2020

Is this possible in NodeJS? You can try in a Node console

Not yet, unfortunately. proposal Staged 3

I have code pieces that use this feature (private getters and setters). They are transpiled by the Babel. Now I cannot rewrite them into TS. :(

NodeJS v12 have support for private class fields because it have V8 v7.4, which implements it even though stills at stage 3

@kitsonk
Copy link
Contributor

kitsonk commented May 13, 2020

While there is some confusion on this, both private fields and private methods are Stage 3. Private fields are allowed in TypeScript, but private methods are not (likely because the methods proposal took longer to get to Stage 3).

In @denoland we have a version of V8 that supports the private methods proposal. It would be great to have private methods as part of esnext target in TypeScript so we can author code in TypeScript that is supported in the runtime.

@TheBrokenRail
Copy link

QuickJS also has support for private methods:

class Test {
    #privateThing() {
        console.log('Private');
    }
    publicThing() {
        this.#privateThing();
    }
}

@Jamesernator
Copy link

Private methods/accessors are now officially shipping in Chrome stable (https://chromestatus.com/feature/5700509656678400) and have been shipping in Node for a couple weeks (as of 14.6.0)

@likern
Copy link

likern commented Aug 11, 2020

I think this should work

class Queue {
  #array = []
  #isEmpty = () => {
    return this.#array.length === 0;
  }
}

@fwienber
Copy link

fwienber commented Aug 18, 2020

Yes, but I guess it leads to #isEmpty being assigned to each instance instead of being assigned to the class prototype like other methods.

@kitsonk
Copy link
Contributor

kitsonk commented Sep 25, 2020

@orta is it really in discussion? It is a spec compliance issue now.

@fwienber
Copy link

Also, the workaround leads to the class having field initializers.
While this is not a bad thing per se, it adds an extra constraint on constructor code: If a class has field initializers, its constructor must call super() as the first statement. If there are no field initializers, the only limitation for statements before the super constructor call is that they may not use this, which is less restrictive.

@fwienber
Copy link

One more annoying thing with the workaround: Forward references do not work.
It should be possible to initialize a private field with a call to a private method. But since with the workaround, TypeScript treats the #private method like a field with an initializer, this only works if the private method is declared before the field using it.

Works in ECMAScript:

class Test {
  #myField = this.#init(3);
  #init(x) { return x + 1; }
}

Error ("Property '#init' is used before its declaration") when trying to represent this with the workaround in TypeScript, and also a run-time error in ECMAScript when trying to instantiate that class ("TypeError: Cannot read private member #init from an object whose class did not declare it"):

class Test {
  #myField = this.#init(3);
  #init = (x) => { return x + 1; }
}

This means you are forced to define helper methods before fields, which might not be one's favorite code style.

Considering all the downsides, in our project, we switched back to (clumsy) symbols / "soft" private members. But we are still eagerly waiting for TypeScript to support #private members and would immediately adapt our code!

@SerkanSipahi
Copy link

Is there any reason why it is not allowed to use private methods in Typescript?

@fwienber
Copy link

Is there any reason why it is not allowed to use private methods in Typescript?

Good question. Especially when you use TSC only for type checking and do the actual compilation with Babel, apart from the error message TS18022, everything seems to work fine.
I just give the approach a go to use @ts-ignore when declaring #private methods. Ugly, but still better than Symbols. All code just using such methods works fine and is correctly type-checked without any @ts-ignore.
Could we probably please have a compiler flag to suppress TS18022?

@kitsonk
Copy link
Contributor

kitsonk commented Nov 21, 2020

Could we probably please have a compiler flag to suppress TS18022?

That doesn't make any sense... This is a spec compliance issue and should just be fixed.

@ryall
Copy link

ryall commented Nov 26, 2020

Yet another issue is setting private identifiers via a class constructor:

class Testing {
  constructor(private #test: string) {} // ERROR: Private identifiers cannot be used as parameters
}

TS playground example of above

@fwienber
Copy link

fwienber commented Nov 26, 2020

Could we probably please have a compiler flag to suppress TS18022?

That doesn't make any sense... This is a spec compliance issue and should just be fixed.

Of course I would prefer a fix, too!
I just guess it is much more effort to implement code generation for that feature, and we only need the type checking part. So for the time being, we use @ts-ignore as a workaround, so my real issue with that workaround is that it is still not possible to ignore a specific error.

@stagefright5
Copy link

Is this issue being addressed in the current roadmap? If not, has this issue been triaged at least?
Please give us an update of some sorts.
Thanks

@fwienber
Copy link

Good news here: #39066 (comment)

@danieltroger
Copy link

danieltroger commented Jan 26, 2021

Pls fix this soon, I don't use typescript but a ts linter and babel ts compilation and it sucks to have red lines everywhere while my code runs perfectly fine.

@mahnunchik
Copy link

I'm looking forward to fix, it's horrible microsoft/vscode#106351

@RyanCavanaugh
Copy link
Member

Implemented in #42458

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests