Conversation

JoshuaKGoldberg

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.

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
ajafff suggested changes Jan 11, 2019
View reviewed changes
ajafff
ajafff suggested changes Jan 11, 2019
View reviewed changes
@weswigham

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.

ajafff
ajafff reviewed Jan 11, 2019
View reviewed changes
(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
ajafff suggested changes Jan 12, 2019
View reviewed changes

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?

ajafff
ajafff reviewed Jan 12, 2019
View reviewed changes
@JoshuaKGoldbergJoshuaKGoldberg mentioned this pull request Jan 13, 2019
Merged
@JoshuaKGoldbergJoshuaKGoldberg mentioned this pull request Jan 14, 2019
Closed
Josh Goldberg added 3 commits January 15, 2019 14:40
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.
Merge branch 'master'
da71ed9
Fixed error number following 'master' merge
b4a9ac2
ajafff
ajafff suggested changes Jan 18, 2019
View reviewed changes
@RyanCavanaugh

cc @ahejlsberg for review

@MaxGraeyMaxGraey mentioned this pull request Jan 30, 2019
Merged
@nicolo-ribaudonicolo-ribaudo mentioned this pull request Feb 28, 2019
Closed
@JoshuaKGoldberg

Ping @ahejlsberg - is there anything that needs to be done here? It'd be nice to have this in 😄

@RyanCavanaughRyanCavanaugh added this to the TypeScript 3.6.0 milestone Jul 12, 2019
@JoshuaKGoldberg

Correction: ping, @weswigham?

@pkoch

@rbuckton Is there anything else that @JoshuaKGoldberg can do to move this forward?

@rbuckton

@typescript-bot user test this inline

@typescript-bot

Heya @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.

@rbuckton

@rbuckton Is there anything else that @JoshuaKGoldberg can do to move this forward?

Nope, it looks ready to go actually, and just in time for 4.6 beta.

@rbucktonrbuckton merged commit b7fee7f into microsoft:main Jan 14, 2022
@JoshuaKGoldbergJoshuaKGoldberg deleted the non-this-super-before-super branch January 14, 2022 14:31
@JoshuaKGoldberg

Fantastic, thanks so much for the reviews & merge @rbuckton!

If any issues come out of this change I'm available to try to fix, if that's helpful.

@NaveedAhmadHematmal

The longest-living PR I have ever seen. 😳

@lukescott

YES!!! Thank you!🎉 🎉 🎉

@callumok2004

🥳🎉🎉

@JoshuaKGoldbergJoshuaKGoldberg mentioned this pull request Jan 24, 2022
Closed
5 tasks
@kitsonkkitsonk mentioned this pull request Feb 14, 2022
Merged
This was referencedMar 3, 2022
Open
Merged
@iJungleboy

Amazing work and perseverance. Awesome!

@EstopaceMA

Man! Awesome. The perseverance! 🎉

@johnnyreillyjohnnyreilly mentioned this pull request Apr 1, 2022
Closed
This was referencedApr 13, 2022
Closed
Merged
@p-szmp-szm mentioned this pull request Apr 19, 2022
Closed
@jlustyjlusty mentioned this pull request Apr 19, 2022
Closed
@AgnesTouletAgnesToulet mentioned this pull request Jun 2, 2022
Merged
@vladboss61

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

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

Congrats, apparently the cake did it's job! 🎉

@gbersac

Guy you've made it reddit programmer humor front page! https://www.reddit.com/r/ProgrammerHumor/comments/waa0lz/if_youre_ever_frustrated_that_your__prs/

@taylus

grats on the PR

@upq

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

@microsoftmicrosoft locked as resolved and limited conversation to collaborators Jul 28, 2022
Sign up for free to subscribe to this conversation on . Already have an account? Sign in.

@ajafff ajafff ajafff requested changes

@weswigham weswigham weswigham approved these changes

@rbuckton rbuckton rbuckton approved these changes

@rbuckton rbuckton

For Uncommitted BugPR for untriaged, rejected, closed or missing bug
PR Backlog
Archived in project
TypeScript 3.7.1

Successfully merging this pull request may close these issues.