fix: class field is not accessible via super #54056

Jack-Works · 2023-04-28T13:59:21Z

fixes #54054
fixes #35314
fixes #54900

Guergeiro · 2023-06-02T09:10:42Z

Just a question, does this handle the JS getter methods? Get methods work with super call in JS.

Jack-Works · 2023-06-03T03:39:56Z

Just a question, does this handle the JS getter methods? Get methods work with super call in JS.

Only class fields are affected. getter and setters work normally.

tests/baselines/reference/esDecorators-classExpression-classSuper.2.errors.txt

ekilah · 2023-07-06T20:22:27Z

I was referred to this fix from my issue:

super keyword allows calling arrow functions defined on the super class, but shouldn't #54900

I believe:

fixes #54900

andrewbranch · 2023-07-07T17:14:16Z

@typescript-bot run dt
@typescript-bot test top100

typescript-bot · 2023-07-07T17:14:18Z

Heya @andrewbranch, I've started to run the diff-based top-repos suite on this PR at 2dc49db. You can monitor the build here.

Update: The results are in!

typescript-bot · 2023-07-07T17:14:18Z

Heya @andrewbranch, I've started to run the parallelized Definitely Typed test suite on this PR at 2dc49db. You can monitor the build here.

Update: The results are in!

typescript-bot · 2023-07-07T19:09:57Z

@andrewbranch Here are the results of running the top-repos suite comparing main and refs/pull/54056/merge:

Something interesting changed - please have a look.

Details

appsmithorg/appsmith

7 of 9 projects failed to build with the old tsc and were ignored

app/client/packages/rts/tsconfig.json

error TS2855: Class field 'serverErrorMessaage' defined by the parent class is not accessible in the child class via super.

microsoft/vscode

5 of 53 projects failed to build with the old tsc and were ignored

src/tsconfig.tsec.json

error TS2855: Class field 'roots' defined by the parent class is not accessible in the child class via super.
- src/vs/workbench/api/common/extHostTesting.ts#L898

typescript-bot · 2023-07-07T19:17:45Z

Hey @andrewbranch, the results of running the DT tests are ready.
Everything looks the same!
You can check the log here.

andrewbranch · 2023-07-07T19:31:20Z

These errors might be interesting to look at.

andrewbranch · 2023-07-07T19:42:37Z

In appsmith, the code presumably works at runtime because the compile target is es6 where class fields aren’t supported. VS Code uses useDefineForClassFields: false which I assume has the same effect for this context. We could choose to only issue this error when class fields will be untransformed, but (1) the code is wrong even if it will technically work, and (2) we don’t know anything about the emit of dependency code, so it would be better to do this consistently. Since we’re past 5.2-beta, I think we should wait until 5.3 to merge this breaking change.

Jack-Works · 2023-07-08T14:55:26Z

In appsmith, the code presumably works at runtime because the compile target is es6 where class fields aren’t supported. VS Code uses useDefineForClassFields: false which I assume has the same effect for this context. We could choose to only issue this error when class fields will be untransformed, but (1) the code is wrong even if it will technically work, and (2) we don’t know anything about the emit of dependency code, so it would be better to do this consistently. Since we’re past 5.2-beta, I think we should wait until 5.3 to merge this breaking change.

Sorry, I'm confused, in what case it can be valid? I cannot think of a setup that defines a class field on the prototype instead of the instance.

andrewbranch · 2023-07-10T14:18:53Z

Hm, I assumed that a quirk of the transformation down to ES2015 would make it work somehow, but I think you’re right. Maybe this stuff isn’t working at all? This is very odd 🤔

sandersn · 2023-07-14T23:56:28Z

Even stranger, the first error, in appsmith, looks like a typo.

sandersn

I agree that this should go into 5.3 to avoid breaking vscode after the 5.2 beta.

In the meantime, I had one small nit about the test, but I'm not 100% what a better solution is.

tests/cases/compiler/classFieldSuperNotAccessible.ts

Co-authored-by: Nathan Shively-Sanders <[email protected]>

- Bumps `typescript` and `ts-expose-internals` to v5.3.3 - Removes the test diagnostic for `noSuperProperty` as the compiler catches this now (microsoft/TypeScript/pull/54056) - Make 1:1 changes for methods and fields - Handles the `NoSubstitutionTemplateLiteral` node from microsoft/TypeScript/pull/55930 Please take a look at the module resolution change. I'm not sure if this will have downstream effects that I couldn't catch in testing. Closes #2610 --------- Co-authored-by: Osyris <[email protected]>

TypeScript/JavaScript uses prototype-based inheritance where fields are stored in a flat structure and not in the scope of a particular class definition in the hierarchy. This means that a field specified by a parent class can be replaced by one specified by one of the children. To signal this behaviour, starting in v5.3.0, the TypeScript compiler prevents a class from assigning values to fields specified by a parent class using "super" (microsoft/TypeScript#54056). This patch, upgrades the TypeScript compiler to the latest version and fixes the remaining issues related to the topic mentioned above that now result in compiler errors. Additionally, it also updates the launch task that runs the compiler to ensure it uses a local version of the compiler, as specified by the dependency graph of the VSCode Extension package. Change-Id: Ib3ef774028ec4587c6b8e5eaa2eb395188e0d5a1

typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Apr 28, 2023

Jack-Works mentioned this pull request Apr 29, 2023

Accessing property on super should be a type error #35314

Closed

typescript-bot added For Backlog Bug PRs that fix a backlog bug and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Apr 29, 2023

sandersn requested review from andrewbranch and navya9singh May 9, 2023 00:11

sandersn assigned andrewbranch May 9, 2023

Jack-Works force-pushed the class-field-super branch 3 times, most recently from 8cd587c to 1a3b36b Compare June 10, 2023 11:30

andrewbranch requested changes Jun 12, 2023

View reviewed changes

tests/baselines/reference/esDecorators-classExpression-classSuper.2.errors.txt Outdated Show resolved Hide resolved

jackofsometrade mentioned this pull request Jul 6, 2023

super keyword allows calling arrow functions defined on the super class, but shouldn't #54900

Closed

Jack-Works force-pushed the class-field-super branch from e66a0ec to 2dc49db Compare July 7, 2023 04:29

andrewbranch approved these changes Jul 7, 2023

View reviewed changes

andrewbranch added the Breaking Change Would introduce errors in existing code label Jul 7, 2023

andrewbranch requested review from sandersn and DanielRosenwasser July 10, 2023 14:19

sandersn approved these changes Jul 14, 2023

View reviewed changes

tests/cases/compiler/classFieldSuperNotAccessible.ts Outdated Show resolved Hide resolved

fix: class field is not accessible via super

4b820cb

Jack-Works force-pushed the class-field-super branch from 2dc49db to 4b820cb Compare July 15, 2023 03:42

sandersn added 2 commits September 22, 2023 16:02

Merge branch 'main' into class-field-super

290a327

hereby format

6c20719

sandersn merged commit 5f818a9 into microsoft:main Sep 22, 2023

Jack-Works deleted the class-field-super branch September 23, 2023 06:55

snovader pushed a commit to EG-A-S/TypeScript that referenced this pull request Sep 23, 2023

fix: class field is not accessible via super (microsoft#54056)

4c9bccf

Co-authored-by: Nathan Shively-Sanders <[email protected]>

andrewbranch mentioned this pull request Sep 26, 2023

Misleading error message for super.instanceProperty #55883

Open

This was referenced Sep 26, 2023

Swap erroneous super. access to this. microsoft/vscode#194221

Merged

No error on super. for inferred class properties in JavaScript #55884

Closed

DanielRosenwasser added this to the TypeScript 5.3.0 milestone Sep 26, 2023

Andarist mentioned this pull request Sep 27, 2023

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

Merged

DanielRosenwasser mentioned this pull request Sep 28, 2023

Indexing/element access on super avoids instance property checks #55899

Open

LouieK22 mentioned this pull request Feb 14, 2024

TypeScript 5.3 roblox-ts/roblox-ts#2617

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: class field is not accessible via super #54056

fix: class field is not accessible via super #54056

Jack-Works commented Apr 28, 2023 •

edited

Loading

Guergeiro commented Jun 2, 2023

Jack-Works commented Jun 3, 2023

ekilah commented Jul 6, 2023 •

edited

Loading

andrewbranch commented Jul 7, 2023

typescript-bot commented Jul 7, 2023 •

edited

Loading

typescript-bot commented Jul 7, 2023 •

edited

Loading

typescript-bot commented Jul 7, 2023

appsmithorg/appsmith

app/client/packages/rts/tsconfig.json

microsoft/vscode

src/tsconfig.tsec.json

typescript-bot commented Jul 7, 2023

andrewbranch commented Jul 7, 2023

andrewbranch commented Jul 7, 2023

Jack-Works commented Jul 8, 2023

andrewbranch commented Jul 10, 2023

sandersn commented Jul 14, 2023

sandersn left a comment

fix: class field is not accessible via super #54056

fix: class field is not accessible via super #54056

Conversation

Jack-Works commented Apr 28, 2023 • edited Loading

Guergeiro commented Jun 2, 2023

Jack-Works commented Jun 3, 2023

ekilah commented Jul 6, 2023 • edited Loading

andrewbranch commented Jul 7, 2023

typescript-bot commented Jul 7, 2023 • edited Loading

typescript-bot commented Jul 7, 2023 • edited Loading

typescript-bot commented Jul 7, 2023

appsmithorg/appsmith

app/client/packages/rts/tsconfig.json

microsoft/vscode

src/tsconfig.tsec.json

typescript-bot commented Jul 7, 2023

andrewbranch commented Jul 7, 2023

andrewbranch commented Jul 7, 2023

Jack-Works commented Jul 8, 2023

andrewbranch commented Jul 10, 2023

sandersn commented Jul 14, 2023

sandersn left a comment

Choose a reason for hiding this comment

Jack-Works commented Apr 28, 2023 •

edited

Loading

ekilah commented Jul 6, 2023 •

edited

Loading

typescript-bot commented Jul 7, 2023 •

edited

Loading

typescript-bot commented Jul 7, 2023 •

edited

Loading