GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Allowed non-this, non-super code before super call in derived classes with property initializers by JoshuaKGoldberg · Pull Request #29374 · microsoft/TypeScript

Conversation

@JoshuaKGoldberg

@JoshuaKGoldberg Josh Ghoulberg 👻 (JoshuaKGoldberg) commented Jan 11, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Starts on #8277 by allowing the non-this, non-super code to be root-level statements in the constructor. This will now be allowed:

class Base { }
class Derived extends Base {
    public prop = true;
    constructor(public paramProp = true) {
        console.log("Hello, world!");
        super();
    }
}

It feels wrong to put a new forEachChild loop in the checker, though in the vast majority of user files this will be a very quick one. Is there a better way to check for a reference to super or this?

Edit 2/28/2020: I've mostly resolved the merge conflicts introduced by both # private fields & useDefineForClassFields , but I'm not confident my approach is still a valid one. I'd greatly appreciate it if someone could confirm I'm on the right track!
Oh, and gulp runtests passes locally (on Windows). I'll try a Mac to see if there's some odd encoding/whitespace behavior with the failing test... ✔️

Edit 3/11/2021: It seems this is fairly close to merging.

Edit 1/13/2022: I bought this PR a birthday cake for its third birthday. https://twitter.com/JoshuaKGoldberg/status/1481654056422567944

Blue and white cake with text 'Happy 3rd birthday, #29374 Bump for PR review please!' and the TypeScript logo. A tall slice is removed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
sqs, vkartaviy, unional, ExE-Boss, robpalme, krozner, celian-rib, strelga, dragomirtitian, Mahmoud-Masri, and 120 more reacted with thumbs up emoji Josh-Cena, Calvin-LL, luiznasciment0, hmmhmmhm, skytomo221, dora1998, gfx, mpyw, tetsugi, NaOH-13, and 122 more reacted with laugh emoji danstiner, ElianCordoba, alexeiaccio, sjoerdbeentjes, wdsrocha, WestDiscGolf, hugopacheco, voliva, luizbaldi, steoneill, and 133 more reacted with hooray emoji ExE-Boss, mathiasgheno, mdevdatta, rvanlaak, akwodkiewicz, bolasblack, oggy107, iqingting, jfortier-haptiq, cvgarciarea, and 6 more reacted with heart emoji ExE-Boss, hmmhmmhm, stepbrobd, martinheidegger, halsk, pojntfx, artoliukkonen, edbighead, DjDeveloperr, qezz, and 13 more reacted with rocket emoji c69-addepar, dpihlaja, hmmhmmhm, kseki, matchaism, s0rthak, halsk, pojntfx, DjDeveloperr, franklanganke, and 10 more reacted with eyes emoji
All reactions
  • 👍 130 reactions
  • 😄 132 reactions
  • 🎉 143 reactions
  • ❤️ 16 reactions
  • 🚀 23 reactions
  • 👀 20 reactions
Josh Goldberg
Allowed non-this, non-super code before super call in derived classes
6845db9 
Fixes microsoft#8277.

It feels wrong to put a new `forEachChild` loop in the checker, though in the vast majority of user files this will be a very quick one. Is there a better way to check for a reference to `super` or `this`?
ajafff
Klaus Meinhardt (ajafff) suggested changes Jan 11, 2019
View reviewed changes
Comment thread src/compiler/checker.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Josh Goldberg
Used new isNewThisScope utility and dummy prop in baselines
dbd3a2a
ajafff
Klaus Meinhardt (ajafff) suggested changes Jan 11, 2019
View reviewed changes
Comment thread src/compiler/utilities.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/compiler/utilities.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Josh Goldberg
Accounted for parameters and get/set accessor bodies
5aa79cd
@weswigham

Wesley Wigham (weswigham) commented Jan 11, 2019

Copy link
Copy Markdown
Member

It feels wrong to put a new forEachChild loop in the checker, though in the vast majority of user files this will be a very quick one. Is there a better way to check for a reference to super or this?

TransformFlags.Super | TransformFlags.ContainsSuper for super at least.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ajafff
Klaus Meinhardt (ajafff) reviewed Jan 11, 2019
View reviewed changes
Comment thread src/compiler/utilities.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Josh Goldberg
Accounted for class extensions
02516cc
JoshuaKGoldberg
Josh Ghoulberg 👻 (JoshuaKGoldberg) commented Jan 12, 2019
View reviewed changes
Comment thread tests/baselines/reference/derivedClassSuperProperties.errors.txt Resolved

Uh oh!

There was an error while loading. Please reload this page.
Josh Goldberg
(node|statement)RefersToSuperOrThis wasn't checking root level scope …
00b4a2b 
…boundaries

```ts
function () {
    return this;
}
```

It was immediately going to `ts.forEachChild` so the statement itself wasn't being counted as a new `this` scope.
ajafff
Klaus Meinhardt (ajafff) suggested changes Jan 12, 2019
View reviewed changes

@ajafff Klaus Meinhardt (ajafff) left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This is getting very complex very fast. I wonder if there's an easier way using the control flow graph?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/compiler/utilities.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/compiler/checker.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/compiler/utilities.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/compiler/utilities.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/compiler/utilities.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
ajafff
Klaus Meinhardt (ajafff) reviewed Jan 12, 2019
View reviewed changes
Comment thread tests/baselines/reference/derivedClassSuperProperties.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
@JoshuaKGoldberg Josh Ghoulberg 👻 (JoshuaKGoldberg) mentioned this pull request Jan 13, 2019
Added error for class properties used within their own declaration #29395
Merged
Josh Goldberg
Better 'references' name and comments; accounted for more method edge…
6d06b21 
… case
JoshuaKGoldberg
Josh Ghoulberg 👻 (JoshuaKGoldberg) commented Jan 14, 2019
View reviewed changes
Comment thread tests/baselines/reference/derivedClassSuperProperties.errors.txt Resolved

Uh oh!

There was an error while loading. Please reload this page.
@JoshuaKGoldberg Josh Ghoulberg 👻 (JoshuaKGoldberg) mentioned this pull request Jan 14, 2019
Always allow code before super call when it does not use "this" #8277
Closed
Josh Goldberg added 3 commits January 15, 2019 14:40
Josh Goldberg
Limited super calls to root-level statements in constructor bodies
ccd52fc 
As per discussion in the issue, it would be ideal to consider any block that always ends up calling to super() the equivalent of a root-level super() statement. This would be valid:

```ts
foo = 1;
constructor() {
    condition() ? super(1) : super(0);
    this.foo;
}
```

...as it would compile to the equivalent of:
```ts
function () {
    condition() ? super(1) : super(0);
    this.foo = 1;
    this.foo;
}

That change would a bit more intense and I'm very timid, so leaving it out of this PR. In the meantime the requirement is that the super() statement must itself be root-level.
Josh Goldberg
Merge branch 'master'
da71ed9
Josh Goldberg
Fixed error number following 'master' merge
b4a9ac2
ajafff
Klaus Meinhardt (ajafff) suggested changes Jan 18, 2019
View reviewed changes
Comment thread src/compiler/utilities.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Josh Goldberg added 2 commits January 20, 2019 20:30
Josh Goldberg
Added decorator test cases
0d91548
Josh Goldberg
Again allowed arrow functions
e4dca14
@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) assigned Wesley Wigham (weswigham) Jan 25, 2019
@RyanCavanaugh

Ryan Cavanaugh (RyanCavanaugh) commented Jan 25, 2019

Copy link
Copy Markdown
Member

cc Anders Hejlsberg (@ahejlsberg) for review
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MaxGraey Max Graey (MaxGraey) mentioned this pull request Jan 30, 2019
Implement calls to 'super()' AssemblyScript/assemblyscript#445
Merged
@nicolo-ribaudo Nicolò Ribaudo (nicolo-ribaudo) mentioned this pull request Feb 28, 2019
@babel/plugin-transform-typescript can insert public property initialization before the call to super. babel/babel#9609
Closed
@JoshuaKGoldberg

Josh Ghoulberg 👻 (JoshuaKGoldberg) commented Apr 2, 2019

Copy link
Copy Markdown
Contributor Author

Ping Anders Hejlsberg (@ahejlsberg) - is there anything that needs to be done here? It'd be nice to have this in 😄
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) added this to the TypeScript 3.6.0 milestone Jul 12, 2019
@JoshuaKGoldberg

Josh Ghoulberg 👻 (JoshuaKGoldberg) commented Jul 16, 2019

Copy link
Copy Markdown
Contributor Author

Correction: ping, Wesley Wigham (@weswigham)?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Josh Goldberg
Merge branch 'master'
c8fc681
@pkoch

Paulo Koch (pkoch) commented Jan 14, 2022

Copy link
Copy Markdown

Ron Buckton (@rbuckton) Is there anything else that Josh Ghoulberg 👻 (@JoshuaKGoldberg) can do to move this forward?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@rbuckton

Ron Buckton (rbuckton) commented Jan 14, 2022

Copy link
Copy Markdown
Contributor

TypeScript Bot (@typescript-bot) user test this inline
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@typescript-bot

TypeScript Bot (typescript-bot) commented Jan 14, 2022

Copy link
Copy Markdown
Contributor

Heya Ron Buckton (@rbuckton), I'm starting to run the inline community code test suite on this PR at 1b3dd6d. Hold tight - I'll update this comment with the log link once the build has been queued.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@rbuckton

Ron Buckton (rbuckton) commented Jan 14, 2022

Copy link
Copy Markdown
Contributor

Ron Buckton (@rbuckton) Is there anything else that Josh Ghoulberg 👻 (@JoshuaKGoldberg) can do to move this forward?

Nope, it looks ready to go actually, and just in time for 4.6 beta.
niki4, electron-space, OmgImAlexis, and ben-biber reacted with thumbs up emoji kachkaev, pkoch, and ben-biber reacted with laugh emoji cherryblossom000, koshikraj, tamaina, brooke-gill, dan-lee, GenbuHase, hmmhmmhm, intrnl, pojntfx, douglance, and 20 more reacted with hooray emoji pkoch, diegoasanch, and ben-biber reacted with rocket emoji
All reactions
  • 👍 4 reactions
  • 😄 3 reactions
  • 🎉 30 reactions
  • 🚀 3 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
rbuckton
Ron Buckton (rbuckton) approved these changes Jan 14, 2022
View reviewed changes
@rbuckton
Ron Buckton (rbuckton) merged commit b7fee7f into microsoft:main Jan 14, 2022
@JoshuaKGoldberg
Josh Ghoulberg 👻 (JoshuaKGoldberg) deleted the non-this-super-before-super branch January 14, 2022 14:31
@JoshuaKGoldberg

Josh Ghoulberg 👻 (JoshuaKGoldberg) commented Jan 14, 2022

Copy link
Copy Markdown
Contributor Author

Fantastic, thanks so much for the reviews & merge Ron Buckton (@rbuckton)!

If any issues come out of this change I'm available to try to fix, if that's helpful.
remnantkevin, capaj, NaveedAhmadHematmal, melihcaliskan, KentoNishi, niki4, styfle, ExE-Boss, electron-space, OmgImAlexis, and 7 more reacted with thumbs up emoji wongmjane, bandoche, crashmax-dev, kachkaev, pkoch, s-tsubokawa-safie, qezz, gavr123456789, DerYeger, ferdaber, and 30 more reacted with hooray emoji PatMyron, caiodomingues, d7mtg, and Brodypen reacted with heart emoji
All reactions
  • 👍 17 reactions
  • 🎉 40 reactions
  • ❤️ 4 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@NaveedAhmadHematmal

Naveed Ahmad Hematmal (NaveedAhmadHematmal) commented Jan 14, 2022

Copy link
Copy Markdown

The longest-living PR I have ever seen. 😳
ExE-Boss, Exac, TkDodo, OmgImAlexis, capaj, lucasgelfond, ben-biber, and DominikKor reacted with thumbs up emoji yvele, MaoDoBan, ben-biber, lucasstarsz, and sym0nd0 reacted with eyes emoji
All reactions
  • 👍 8 reactions
  • 👀 5 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@lukescott

Luke Scott (lukescott) commented Jan 20, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

YES!!! Thank you!🎉 🎉 🎉
Techie-Pi, JoshuaKGoldberg, ben-biber, and indicozy reacted with hooray emoji JoshuaKGoldberg and ben-biber reacted with heart emoji
All reactions
  • 🎉 4 reactions
  • ❤️ 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@callumok2004

Callum (callumok2004) commented Jan 22, 2022

Copy link
Copy Markdown

🥳🎉🎉
JoshuaKGoldberg, demensky, yvele, kkoscielniak, ben-biber, and indicozy reacted with hooray emoji
All reactions
  • 🎉 6 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@JoshuaKGoldberg Josh Ghoulberg 👻 (JoshuaKGoldberg) mentioned this pull request Jan 24, 2022
Remove older emit support over time #47573
Closed
5 tasks done
@kitsonk Kitson Kelly (kitsonk) mentioned this pull request Feb 14, 2022
fix(io/buffer): super and initialized prop denoland/std#1917
Merged
This was referenced Mar 3, 2022
Argument becomes implicit any when a void-returning overload follows a non-void-returning signature #48088
Open
Blog post: code-before-super JoshuaKGoldberg/Goldblog#335
Merged
@iJungleboy

iJungleboy (iJungleboy) commented Mar 7, 2022

Copy link
Copy Markdown

Amazing work and perseverance. Awesome!
JoshuaKGoldberg and ben-biber reacted with heart emoji
All reactions
  • ❤️ 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@EstopaceMA

anth.dev (EstopaceMA) commented Mar 18, 2022

Copy link
Copy Markdown

Man! Awesome. The perseverance! 🎉
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@johnnyreilly John Reilly (johnnyreilly) mentioned this pull request Apr 1, 2022
[RFC] Towards Non-Fungible Types #48513
Closed
This was referenced Apr 13, 2022
No-op string in constructor is repeated and reordered when constructor assignment is used #48671
Closed
Fixed multiple prologue directives with parameter properties #48687
Merged
@p-szm P Sz (p-szm) mentioned this pull request Apr 19, 2022
Wrong constructor code generated when target is ES5 #48761
Closed
@jlusty John Lusty (jlusty) mentioned this pull request Apr 19, 2022
Missing parameter properties when using class with private fields #48771
Closed
@AgnesToulet Agnès Toulet (AgnesToulet) mentioned this pull request Jun 2, 2022
Repository Dispatch: improve error message for fork PRs grafana/grafana-github-actions#97
Merged
@vladboss61

Vladyslav V (vladboss61) commented Jul 7, 2022

Copy link
Copy Markdown

Does it mean that now super is going to be invoked the first time never mind where he is located in the constructor?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Eemeli Aro (eemeli) added a commit to messageformat/messageformat that referenced this pull request Jul 16, 2022
@eemeli
fix(mf2): Use private member for ResolvedMessage cache, thanks to mic…
0fbf543 
…rosoft/TypeScript#29374

The minimum TypeScript dependency is lifted to 4.6.2
@jfortier-haptiq

Justin Fortier (jfortier-haptiq) commented Jul 28, 2022

Copy link
Copy Markdown

Congrats, apparently the cake did it's job! 🎉
ate47, ben-biber, lucasstarsz, vanjmali, ddvdozuki, Dummiesman, RodPaDev, and tweimann reacted with thumbs up emoji adrianodennanni, suy, lmaydev, dpeter99, Snapstromegon, and lboje reacted with thumbs down emoji
All reactions
  • 👍 8 reactions
  • 👎 6 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gbersac

Guillaume Bersac (gbersac) commented Jul 28, 2022

Copy link
Copy Markdown

Guy you've made it reddit programmer humor front page! https://www.reddit.com/r/ProgrammerHumor/comments/waa0lz/if_youre_ever_frustrated_that_your_github_prs/
Dummiesman, sym0nd0, taylus, Ensber, ShazimC, blyxyas, DominikKor, and tweimann reacted with thumbs up emoji adrianodennanni, rmnscnce, zopad, maiatteo, boehs, itsthejoker, TechnologicNick, dabjulmaros, Alexander-Wilms, codeocelot, and 10 more reacted with thumbs down emoji leviathanaxeislit, vanjmali, charles-hay, maiatteo, Dummiesman, Aarekaz, suy, and tweimann reacted with laugh emoji
All reactions
  • 👍 8 reactions
  • 👎 20 reactions
  • 😄 8 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@taylus

taylus commented Jul 28, 2022

Copy link
Copy Markdown

grats on the PR
lmaydev, dpeter99, ShroomAgent27, Congee, Snapstromegon, and lboje reacted with thumbs down emoji
All reactions
  • 👎 6 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@upq

upq commented Jul 28, 2022

Copy link
Copy Markdown

Congrats !! Happy for you!
Might as well be the most famous PR yet ... from branding perspective I recommend you naming the PR
Suggestion: Des Supa
Snapstromegon and lboje reacted with thumbs down emoji
All reactions
  • 👎 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Jul 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@weswigham Wesley Wigham (weswigham) weswigham approved these changes
+2 more reviewers
@ajafff Klaus Meinhardt (ajafff) ajafff requested changes
@rbuckton Ron Buckton (rbuckton) rbuckton approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@rbuckton Ron Buckton (rbuckton)

Labels

For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

PR Backlog
Archived in project

Milestone

TypeScript 3.7.1

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

20 participants

@JoshuaKGoldberg @weswigham @RyanCavanaugh @DanielRosenwasser @typescript-bot @andrewbranch @rbuckton @pkoch @NaveedAhmadHematmal @lukescott @callumok2004 @iJungleboy @EstopaceMA @vladboss61 @jfortier-haptiq @gbersac @taylus @upq @ajafff @sandersn
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.