Error on class fields accesses through super in JS files#55892
Conversation
| !!! error TS2855: Class field 'justProp' defined by the parent class is not accessible in the child class via super. | ||
| } | ||
| get literalElementAccessTests() { | ||
| return super.literalElementAccess; |
There was a problem hiding this comment.
Can you add a test for super["literalElementAccess"] as well?
Also, can you just add a test for super.b? I bet we have no test for super. on an accessor declaration at all.
There was a problem hiding this comment.
Can you add a test for super["literalElementAccess"] as well?
Good catch. It doesn't work correctly today (TS playground) even in TS files. For that reason - I'd say that this is a separate issue. I can look into it as well but I'd prefer to explore it separately.
Also, can you just add a test for super.b? I bet we have no test for super. on an accessor declaration at all.
Sure thing, just added it.
|
Can you add a test for this case in JavaScript as well and confirm it works? // @checkJs: true
// @filename: foo.js
export class C {
static blah1 = 123;
}
C.blah2 = 456;
export class D extends C {
static {
console.log(super.blah1);
console.log(super.blah2);
}
}along with this: // @checkJs: true
// @filename: foo.js
class C {
constructor() {
this.foo = () => {
console.log("called arrow")
}
}
foo() {
console.log("called method")
}
}
class D extends C {
foo() {
console.log("SUPER:");
super.foo();
console.log("THIS:");
this.foo();
}
}
const obj = new D();
obj.foo();
D.prototype.foo.call(obj); |
Added both - I had to fix one of them 😉 |
|
@typescript-bot test top100 |
|
Heya @jakebailey, I've started to run the diff-based top-repos suite on this PR at 23ec307. You can monitor the build here. Update: The results are in! |
|
Heya @jakebailey, I've started to run the diff-based user code test suite on this PR at 23ec307. You can monitor the build here. Update: The results are in! |
|
@jakebailey Here are the results of running the user test suite comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Everything looks good! |
|
|
||
| /** @internal */ | ||
| export function isClassFieldAndNotAutoAccessor(node: Node): boolean { | ||
| export function isClassFieldAndNotAutoAccessor(node: Declaration): boolean { |
There was a problem hiding this comment.
| export function isClassFieldAndNotAutoAccessor(node: Declaration): boolean { | |
| export function isClassInstanceProperty(node: Declaration): boolean { |
|
@jakebailey Here are the results of running the top-repos suite comparing Everything looks good! |
|
Let's get this in for 5.3 beta and do the rename after. |
Andarist
deleted the
fix/inferred-class-fields-in-js-not-through-super
branch
4 participants
fixes #55884
follow up to #54056