-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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 members are accessible or inaccessible on this
depending on how this
is defined
#56094
Comments
That the first one works at all looks like a bug. |
To be clear, are you requesting that both are an error, or that neither is an error? |
@RyanCavanaugh I am requesting that neither are an error. If both would be an error, there would be no way to use a type-safe mixin pattern, where functions are intended to be added dynamically as "methods" of a class when the constructor is called, because those functions would have no access to protected members. (Unless there's another way to designate this type in TypeScript?) For context, the reason this pattern exists is for use with the Chevrotain parsing library. Methods are "attached" dynamically when the constructor is called, because Chevrotain "records" parsing rules in order to do lookahead prediction. Nowadays, you could probably use something like decorators on regular class methods (maybe?), but neither decorators nor TypeScript had a foothold when Chevrotain was created, AFAIK. So, currently, instance methods are created, and there is an expectation in the API that those methods have access to protected class methods, because they are bound to and run in the context of the class instance. (See: https://chevrotain.io/docs/tutorial/step2_parsing.html#structure. |
If it were considered to be, and this "bug" were "fixed", it would destroy all of the types in my project. |
For what it's worth this only works with class Foo {
private field = 'bar'
copy!: string
constructor() {
bindCopy.call(this)
bindCopy2.call(this)
}
}
function bindCopy(this: Foo) {
this.copy = this.field // this is an error now too
} which I'm pretty sure was reported a while back and was closed working as intended. |
The correct behavior is to allow external functions to access class Dummy extends Foo {
blah() {
return this.protectedMember;
}
}
// Does what you want
const haha_yes = (new Dummy()).blah;
|
🔎 Search Terms
class members protected only accessible
🕗 Version & Regression Information
⏯ Playground Link
https://www.typescriptlang.org/play?#code/MYGwhgzhAEBiD29oG8BQ1oAcBO8AuApsIQCbQBmAlgSGQLzQDkARmNo+tMPJgJ4CEALmgQ82SgDsA5p24TR2AK7F42ABQBKFJwzNJJAMI9eAOmBgQINXgAWlCBp3Q9Ew8YBMZi1dv3HGAF9UINRyRQliSngJZ30jPms7CGEEeC00DF8IM2NoBiyTKhoSYNRUPF5MAmgAIX1JKVhwyOi86ET7FMQtOgA+aAA3eEoS1DlRWNd43ndhOtcGpoi8KJiGMOXVzW1MpJy+NoKi2mCgA
💻 Code
🙁 Actual behavior
Second definition (
bindCopy2
) throws:🙂 Expected behavior
Both definitions have the same
this
binding. One is simply abstracted into a type. Both should pass similarly.Additional information about the issue
I recently wanted to refactor some functions I had to bind values in a class's constructor. Note that while this code looks simple, it should be noted that the actual functions CANNOT be methods on the class instance, because they function as "mixins" on multiple classes.
I naively assumed I could just define a type for each function, and thus eliminate the
this
declaration in each binding function. However, to my surprise, moving the type ofthis
to a separatetype
definition caused theprotected
error to be thrown in the function.Because the function is identical, and the value of
this
is identical, it is not intuitive why one form is allowed and the other isn't.The text was updated successfully, but these errors were encountered: