Skip to content

Add support for captured block scoped bindings #7693

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

Merged
merged 5 commits into from
Mar 29, 2016

Conversation

vladima
Copy link
Contributor

@vladima vladima commented Mar 26, 2016

// cc @rbuckton

@@ -62,10 +195,19 @@ namespace ts {
const savedEnclosingBlockScopeContainer = enclosingBlockScopeContainer;
const savedEnclosingBlockScopeContainerParent = enclosingBlockScopeContainerParent;

const savedConvertedLoopState = convertedLoopState;
if (nodeStartsNewLexicalEnvironment(node) || isClassLike(node)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What about an arrow function in a computed property name or extends clause of a class?

@vladima
Copy link
Contributor Author

vladima commented Mar 27, 2016

Good point

onBeforeVisitNode(node);
const savedConvertedLoopState = convertedLoopState;
if (nodeStartsNewLexicalEnvironment(node)) {
// don't treat content of nodes that start new lexical environment or class-like nodes as part of converted loop copy
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment may now be incorrect.

How do we want to handle captured block scope bindings for initializers?

let ar = [];
for (let i = 0; i < 2; i++) {
  ar.push(class { x = i; });
}
console.log(new ar[0]().x); // should this be 0 or 2?
console.log(new ar[1]().x); // should this be 1 or 2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

comment is fixed. Re initializers, I think this code should be equivalent to

"use strict"
let ar = [];
for (let i = 0; i < 2; i++) {
  ar.push(class { x: number; constructor() { this.x=i }; });
}
console.log(new ar[0]().x); // should be 0
console.log(new ar[1]().x); // should be 1

old emitter does not support this either so I'll file it as a separate issue that should be fixed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

filed #7714 to track it

@rbuckton
Copy link
Contributor

👍

nodeIsSynthesized(statements[0]) ||
rangeStartPositionsAreOnSameLine(parentNode, statements[0], currentSourceFile)
);
if (emitAsSingleStatement) {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Newline above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vladima vladima merged commit 3989d23 into transforms Mar 29, 2016
@vladima vladima deleted the transforms-block-scoped-bindings branch March 29, 2016 16:39
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants