Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use returned values from super calls as 'this' #10762

Merged
merged 44 commits into from
Sep 30, 2016

Conversation

DanielRosenwasser
Copy link
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.

…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.
@andreawyss
Copy link

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

@DanielRosenwasser DanielRosenwasser force-pushed the useReturnedThisFromSuperCalls branch from 36a5c42 to b5a1031 Compare September 27, 2016 21:45
@DanielRosenwasser
Copy link
Member Author

DanielRosenwasser commented Sep 27, 2016

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

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

@sandersn
Copy link
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.

@DanielRosenwasser
Copy link
Member Author

@sandersn are you sure you don't see a tests/cases/compiler/derivedClassConstructorWithExplicitReturns01.ts?

@sandersn
Copy link
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.

@rbuckton
Copy link
Member

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

@DanielRosenwasser
Copy link
Member Author

@rbuckton nope, no changes.

@DanielRosenwasser DanielRosenwasser merged commit bcdf1dc into master Sep 30, 2016
@DanielRosenwasser DanielRosenwasser deleted the useReturnedThisFromSuperCalls branch September 30, 2016 07:08
@DanielRosenwasser
Copy link
Member Author

Thanks @rbuckton & @sandersn!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Breaking Change Would introduce errors in existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return value of super() calls not used for this
6 participants