GeezerVison

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

GeezerVison is showing github.com — Use returned values from super calls as 'this' by DanielRosenwasser · Pull Request #10762 · microsoft/TypeScript

Conversation

Daniel Rosenwasser (DanielRosenwasser) commented Sep 7, 2016

Copy link

Copy Markdown

Member

This PR fixes #7574.

In ES2015, constructors which return a value (which is an object) implicitly substitute the value of this for any callers of super(). As a result, it is necessary to capture any potential return value of super() and replace it with this.

Some remaining work might be to do some refactoring on the emit code.

I'd suggest reading one commit at a time.

Sorry, something went wrong.

Uh oh!

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

HerringtonDarkholme, rictic, Hotell, and r3dsm0k3 reacted with thumbs up emoji

GoToLoop, rictic, and r3dsm0k3 reacted with hooray emoji

All reactions

👍 4 reactions
🎉 3 reactions

Daniel Rosenwasser (DanielRosenwasser) added 21 commits

September 1, 2016 18:15


          Emit '_this' declaration for derived classes; initialize it when call…

9dbab97

…ing 'super(...)'.


          Accepted baselines.

99f1df4


          Transform other instances of 'this' to '_this' when in the constructo…

d778e78

…r of a derived class.


          Accepted baselines for all tests apart from one with source map changes.

c21cc24


          Accepted baselines for sourcemap test.

9c6e148


          Always tack on a return statement for '_this' in derived classes.

cd5e76b


          Accepted baselines.

30ec599


          Ensure prologue directives occur first.

bab6d6f


          Accepted baselines.

ce953ce


          Consolidate '_this' declaration and assignments for first statements …

d6e548b

…in constructors.


          Fixed issue where last function context & parent node wasn't being pr…

11bc6c4

…eserved.

This came up when a `super()` call was nested in another constructor body.
Current logic in the transform says that if the last containing non-arrow
function body is non-static, and the current parent isn't a call expression,
the call target of a `super` call will become `_super.prototype` instead
of `super`.

If the state is not saved, the containing arrow function and parent
are not saved, and the information for this check won't be accurate.


          Accepted baselines.

d8846dd


          Always perform this captures after default & rest parameters.

86088d0


          Accepted baselines.

dd27139


          Added a test for branched returns at the end of a constructor.

f3dcaae


          Accepted baselines.

1be4cee


          Don't emit a return statement at the end in most useful cases.

c87a773


          Accepted baselines.

d144665


          Added test for '_this'.

b476815


          Accepted baselines.

230737f


          Mark constructors as this-capturing if they are defined in a derived …

b11db57

…class.

Microsoft Contribution License Agreements (msftclas) added the cla-not-required label

Sep 7, 2016

Daniel Rosenwasser (DanielRosenwasser) added 3 commits

September 7, 2016 15:42


          Merge branch 'master' into useReturnedThisFromSuperCalls

25f9555


          Accepted baselines.

3a5fb0c


          Added missing semicolons.

f11c646

Daniel Rosenwasser (DanielRosenwasser) mentioned this pull request

Sep 9, 2016

Design Meeting Notes, 9/9/2016 #10826

Closed

Andy R. Wyss (andreawyss) commented Sep 15, 2016

Copy link

Copy Markdown

Awesome work!
I have verified, that with this branch of TypeScript, I can now in fact create CustomElements entirely in TypeScript. Thank You
FYI: Justin Fagnani (@justinfagnani) Trey Shugart (@treshugart)

DanielRosenwasser and Hotell reacted with hooray emoji

All reactions

🎉 2 reactions

Sorry, something went wrong.

Uh oh!

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

Andy R. Wyss (andreawyss) mentioned this pull request

Sep 15, 2016

Allows to pass to reactify the tagName instead of the CustomElement webcomponents/react-integration#38

Closed

Daniel Rosenwasser (DanielRosenwasser) added 2 commits

September 19, 2016 17:47


          Merge remote-tracking branch 'origin/master' into useReturnedThisFrom…

a63a0d8

…SuperCalls


          Accepted baselines.

138e18c

Daniel Rosenwasser (DanielRosenwasser) added 7 commits

September 27, 2016 11:44


          Remove usage of 'useCapturedThis'.

4a5830d


          Collapse 'super()' capture/returns into a single return statement for…

c668644

… generated 'super()' calls.


          Accepted baselines.

1fbdb86


          Restore arrow function.

e0c35f2


          Reuse the 'captureThisForNode' function.

f8fbc22


          Removed pointlessish overloads.

fb4b503


          Moved code around, fixed indent, reworded comment.

b5a1031

Daniel Rosenwasser (DanielRosenwasser) force-pushed the useReturnedThisFromSuperCalls branch from 36a5c42 to b5a1031 Compare

September 27, 2016 21:45

Daniel Rosenwasser (DanielRosenwasser) commented Sep 27, 2016 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Member Author

I don't think our test harness gives an error for this right now.

Nathan Shively-Sanders (@sandersn) how have we been operating like this? In any case, it looks like GitHub just has a limitation and might not be showing that file. I seem to have it locally.

All reactions

Sorry, something went wrong.

Uh oh!

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

Nathan Shively-Sanders (sandersn) commented Sep 27, 2016

Copy link

Copy Markdown

Member

It's only been since we made the tests run faster by not writing output for passing tests.

I have the PR open in codeflow too or I would not have noticed the missing file at all. But maybe it's missing at the API level, not just the UI level.

All reactions

Sorry, something went wrong.

Uh oh!

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


          Consolidated 'super()' transformation logic into one function.

c29ec6f

Daniel Rosenwasser (DanielRosenwasser) commented Sep 28, 2016

Copy link

Copy Markdown

Member Author

Nathan Shively-Sanders (@sandersn) are you sure you don't see a tests/cases/compiler/derivedClassConstructorWithExplicitReturns01.ts?

All reactions

Sorry, something went wrong.

Uh oh!

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

Nathan Shively-Sanders (sandersn) commented Sep 28, 2016

Copy link

Copy Markdown

Member

When I check out the branch myself I see it, but I don't find it in the github list of files either. I guess it's a limitation of github proper, not just its UI.

All reactions

Sorry, something went wrong.

Uh oh!

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

Ron Buckton (rbuckton) commented Sep 30, 2016

Copy link

Copy Markdown

Contributor

Daniel Rosenwasser (@DanielRosenwasser) after the removal of useCapturedThis did you see any changes in the test results?

All reactions

Sorry, something went wrong.

Uh oh!

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


          Merge remote-tracking branch 'origin/master' into useReturnedThisFrom…

02b9917

…SuperCalls

Daniel Rosenwasser (DanielRosenwasser) commented Sep 30, 2016

Copy link

Copy Markdown

Member Author

Ron Buckton (@rbuckton) nope, no changes.

All reactions

Sorry, something went wrong.

Uh oh!

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

Ron Buckton (rbuckton) approved these changes

Sep 30, 2016

View reviewed changes

Daniel Rosenwasser (DanielRosenwasser) merged commit bcdf1dc into master

Sep 30, 2016

Daniel Rosenwasser (DanielRosenwasser) deleted the useReturnedThisFromSuperCalls branch

September 30, 2016 07:08

Daniel Rosenwasser (DanielRosenwasser) commented Sep 30, 2016

Copy link

Copy Markdown

Member Author

Thanks Ron Buckton (@rbuckton) & Nathan Shively-Sanders (@sandersn)!

bradleyayers reacted with hooray emoji

All reactions

🎉 1 reaction

Sorry, something went wrong.

Uh oh!

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

Herrington Darkholme (HerringtonDarkholme) mentioned this pull request

Nov 10, 2016

Incorrect transpiled constructor of derived classes #12123

Closed

Daniel Rosenwasser (DanielRosenwasser) mentioned this pull request

Nov 23, 2016

#931 - Use Flow for Development skatejs/skatejs#931

Closed

kiara (KiaraGrouwstra) mentioned this pull request

Nov 30, 2016

Add typescript supports fantasyland/fantasy-land#140

Closed

Mohamed Hegazy (mhegazy) added the Breaking Change Would introduce errors in existing code label

Dec 2, 2016

Mohamed Hegazy (mhegazy) added this to the TypeScript 2.1 milestone

Dec 2, 2016

Masahiro Wakame (vvakame) mentioned this pull request

Mar 17, 2017

Subclass extends Error is broken #14702

Closed

Ryan Cavanaugh (RyanCavanaugh) mentioned this pull request

Mar 27, 2017

a custom class extends to Object, invoke a member function will produce an error!!! #14869

Closed

Daniel Rosenwasser (DanielRosenwasser) mentioned this pull request

Sep 19, 2017

Runtime error when mixing into Object with es5 target #18564

Closed

Bryan Smith (bryanrsmith) mentioned this pull request

Feb 9, 2018

Unexpected super() return value when subclassing ES5 ctor function chakra-core/ChakraCore#4663

Closed

Microsoft (microsoft) locked and limited conversation to collaborators

Jun 19, 2018

Reviewers

Nathan Shively-Sanders (sandersn)

sandersn left review comments

+1 more reviewer

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

No one assigned

Labels

Breaking Change

Would introduce errors in existing code

Projects

None yet

Milestone

TypeScript 2.1

Development

Successfully merging this pull request may close these issues.

Return value of super() calls not used for this

Uh oh!

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

6 participants

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.

GeezerVison

Conversation

Daniel Rosenwasser (DanielRosenwasser) commented Sep 7, 2016

Uh oh!

Andy R. Wyss (andreawyss) commented Sep 15, 2016

Uh oh!

Daniel Rosenwasser (DanielRosenwasser) commented Sep 27, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nathan Shively-Sanders (sandersn) commented Sep 27, 2016

Uh oh!

Daniel Rosenwasser (DanielRosenwasser) commented Sep 28, 2016

Uh oh!

Nathan Shively-Sanders (sandersn) commented Sep 28, 2016

Uh oh!

Ron Buckton (rbuckton) commented Sep 30, 2016

Uh oh!

Daniel Rosenwasser (DanielRosenwasser) commented Sep 30, 2016

Uh oh!

Daniel Rosenwasser (DanielRosenwasser) commented Sep 30, 2016

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Daniel Rosenwasser (DanielRosenwasser) commented Sep 27, 2016 •

edited

Loading

Uh oh!

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