Skip to content

Error on class fields accesses through super in JS files - #55892

Merged
Daniel Rosenwasser (DanielRosenwasser) merged 5 commits into
microsoft:mainfrom
Andarist:fix/inferred-class-fields-in-js-not-through-super
Sep 29, 2023
Merged

Error on class fields accesses through super in JS files#55892
Daniel Rosenwasser (DanielRosenwasser) merged 5 commits into
microsoft:mainfrom
Andarist:fix/inferred-class-fields-in-js-not-through-super

Conversation

@Andarist

Copy link
Copy Markdown
Contributor

fixes#55884
follow up to #54056

@typescript-botTypeScript Bot (typescript-bot) added the For Backlog Bug PRs that fix a backlog bug label Sep 27, 2023
Comment threadsrc/compiler/utilitiesPublic.ts Outdated
!!! error TS2855: Class field 'justProp' defined by the parent class is not accessible in the child class via super.
}
get literalElementAccessTests() {
return super.literalElementAccess;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@DanielRosenwasser

Copy link
Copy Markdown
Member

Can you add a test for this case in JavaScript as well and confirm it works?

// @checkJs: true// @filename: foo.jsexportclassC{staticblah1=123;}C.blah2=456;exportclassDextendsC{static{console.log(super.blah1);console.log(super.blah2);}}

along with this:

// @checkJs: true// @filename: foo.jsclassC{constructor(){this.foo=()=>{console.log("called arrow")}}foo(){console.log("called method")}}classDextendsC{foo(){console.log("SUPER:");super.foo();console.log("THIS:");this.foo();}}constobj=newD();obj.foo();D.prototype.foo.call(obj);

@Andarist

Copy link
Copy Markdown
ContributorAuthor

Can you add a test for this case in JavaScript as well and confirm it works?

Added both - I had to fix one of them 😉

@jakebailey

Copy link
Copy Markdown
Member

@typescript-bot

TypeScript Bot (typescript-bot) commented Sep 29, 2023

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@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!

@typescript-bot

TypeScript Bot (typescript-bot) commented Sep 29, 2023

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@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!

@typescript-bot

Copy link
Copy Markdown
Contributor

Jake Bailey (@jakebailey) Here are the results of running the user test suite comparing main and refs/pull/55892/merge:

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Unknown failure"
  • 2 instances of "Package install failed"

Otherwise...

Everything looks good!


/** @internal */
export function isClassFieldAndNotAutoAccessor(node: Node): boolean {
export function isClassFieldAndNotAutoAccessor(node: Declaration): boolean {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
exportfunctionisClassFieldAndNotAutoAccessor(node: Declaration): boolean{
exportfunctionisClassInstanceProperty(node: Declaration): boolean{

@typescript-bot

Copy link
Copy Markdown
Contributor

Jake Bailey (@jakebailey) Here are the results of running the top-repos suite comparing main and refs/pull/55892/merge:

Everything looks good!

@DanielRosenwasser

Copy link
Copy Markdown
Member

Let's get this in for 5.3 beta and do the rename after.

@Andarist
Mateusz Burzyński (Andarist) deleted the fix/inferred-class-fields-in-js-not-through-super branch September 29, 2023 22:04
@microsoftMicrosoft (microsoft) locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

For Backlog BugPRs that fix a backlog bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No error on super. for inferred class properties in JavaScript

4 participants

@Andarist@DanielRosenwasser@jakebailey@typescript-bot