From 8fa44c3b06c798b6d4399d7836320df38f5fb4ec Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 13 Apr 2016 15:49:02 -0700 Subject: [PATCH 1/3] Capture `this` in computed property names in arrow functions --- src/compiler/binder.ts | 12 ++++++++++++ src/compiler/types.ts | 15 ++++++++------- .../reference/computedPropertyNames29_ES5.js | 3 ++- .../reference/computedPropertyNames31_ES5.js | 3 ++- 4 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index 1cff02ffc48..b48b320d0d1 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -1910,6 +1910,9 @@ namespace ts { // This is so that they can flow through PropertyName transforms unaffected. // Instead, we mark the container as ES6, so that it can properly handle the transform. transformFlags = TransformFlags.ContainsComputedPropertyName; + if (subtreeFlags & TransformFlags.ContainsLexicalThis) { + transformFlags |= TransformFlags.ContainsLexicalThisInComputedPropertyName; + } break; case SyntaxKind.SpreadElementExpression: @@ -1945,6 +1948,9 @@ namespace ts { // is an ES6 node. transformFlags = TransformFlags.AssertES6; } + if (subtreeFlags & TransformFlags.ContainsLexicalThisInComputedPropertyName) { + transformFlags |= TransformFlags.ContainsLexicalThis; + } break; case SyntaxKind.CallExpression: @@ -2256,6 +2262,9 @@ namespace ts { || hasModifier(node, ModifierFlags.Export)) { transformFlags |= TransformFlags.AssertTypeScript; } + if (subtreeFlags & TransformFlags.ContainsLexicalThisInComputedPropertyName) { + transformFlags |= TransformFlags.ContainsLexicalThis; + } return updateTransformFlags(node, subtreeFlags, transformFlags, TransformFlags.ClassExcludes); } @@ -2272,6 +2281,9 @@ namespace ts { | TransformFlags.ContainsDecorators)) { transformFlags |= TransformFlags.AssertTypeScript; } + if (subtreeFlags & TransformFlags.ContainsLexicalThisInComputedPropertyName) { + transformFlags |= TransformFlags.ContainsLexicalThis; + } return updateTransformFlags(node, subtreeFlags, transformFlags, TransformFlags.ClassExcludes); } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 627b33f0d34..4c99417950d 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2830,11 +2830,12 @@ namespace ts { ContainsPropertyInitializer = 1 << 10, ContainsLexicalThis = 1 << 11, ContainsCapturedLexicalThis = 1 << 12, - ContainsDefaultValueAssignments = 1 << 13, - ContainsParameterPropertyAssignments = 1 << 14, - ContainsSpreadElementExpression = 1 << 15, - ContainsComputedPropertyName = 1 << 16, - ContainsBlockScopedBinding = 1 << 17, + ContainsLexicalThisInComputedPropertyName = 1 << 13, + ContainsDefaultValueAssignments = 1 << 14, + ContainsParameterPropertyAssignments = 1 << 15, + ContainsSpreadElementExpression = 1 << 16, + ContainsComputedPropertyName = 1 << 17, + ContainsBlockScopedBinding = 1 << 18, HasComputedFlags = 1 << 31, // Transform flags have been computed. @@ -2853,10 +2854,10 @@ namespace ts { FunctionExcludes = ContainsDecorators | ContainsDefaultValueAssignments | ContainsCapturedLexicalThis | ContainsLexicalThis | ContainsParameterPropertyAssignments | ContainsBlockScopedBinding, ConstructorExcludes = ContainsDefaultValueAssignments | ContainsLexicalThis | ContainsCapturedLexicalThis | ContainsBlockScopedBinding, MethodOrAccessorExcludes = ContainsDefaultValueAssignments | ContainsLexicalThis | ContainsCapturedLexicalThis | ContainsBlockScopedBinding, - ClassExcludes = ContainsDecorators | ContainsPropertyInitializer | ContainsLexicalThis | ContainsCapturedLexicalThis | ContainsComputedPropertyName | ContainsParameterPropertyAssignments, + ClassExcludes = ContainsDecorators | ContainsPropertyInitializer | ContainsLexicalThis | ContainsCapturedLexicalThis | ContainsComputedPropertyName | ContainsParameterPropertyAssignments | ContainsLexicalThisInComputedPropertyName, ModuleExcludes = ContainsDecorators | ContainsLexicalThis | ContainsCapturedLexicalThis | ContainsBlockScopedBinding, TypeExcludes = ~ContainsTypeScript, - ObjectLiteralExcludes = ContainsDecorators | ContainsComputedPropertyName, + ObjectLiteralExcludes = ContainsDecorators | ContainsComputedPropertyName | ContainsLexicalThisInComputedPropertyName, ArrayLiteralOrCallOrNewExcludes = ContainsSpreadElementExpression, } diff --git a/tests/baselines/reference/computedPropertyNames29_ES5.js b/tests/baselines/reference/computedPropertyNames29_ES5.js index af6f93c8ad9..966081ae035 100644 --- a/tests/baselines/reference/computedPropertyNames29_ES5.js +++ b/tests/baselines/reference/computedPropertyNames29_ES5.js @@ -18,7 +18,8 @@ var C = (function () { var _this = this; (function () { var obj = (_a = {}, - _a[_this.bar()] = function () { }, + _a[_this.bar()] = function () { } // needs capture + , _a); var _a; }); diff --git a/tests/baselines/reference/computedPropertyNames31_ES5.js b/tests/baselines/reference/computedPropertyNames31_ES5.js index 86fb6655a27..2e38e2a131f 100644 --- a/tests/baselines/reference/computedPropertyNames31_ES5.js +++ b/tests/baselines/reference/computedPropertyNames31_ES5.js @@ -38,7 +38,8 @@ var C = (function (_super) { var _this = this; (function () { var obj = (_a = {}, - _a[_super.prototype.bar.call(_this)] = function () { }, + _a[_super.prototype.bar.call(_this)] = function () { } // needs capture + , _a); var _a; }); From e5e8c6b0b936d07365bfcc07b7363380998da163 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 13 Apr 2016 16:14:29 -0700 Subject: [PATCH 2/3] Add explanatory comment when adding ContainsLexicalThisInComputedPropertyName --- src/compiler/binder.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index b48b320d0d1..5ed1e9665d5 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -1911,6 +1911,9 @@ namespace ts { // Instead, we mark the container as ES6, so that it can properly handle the transform. transformFlags = TransformFlags.ContainsComputedPropertyName; if (subtreeFlags & TransformFlags.ContainsLexicalThis) { + // A computed method name that contains `this` needs to + // distinguish itself from the normal case of a method body containing `this`. + // So convert ContainsLexicalThis to ContainsLexicalThisInComputedPropertyName transformFlags |= TransformFlags.ContainsLexicalThisInComputedPropertyName; } break; From 0d5bf0ee32a39288cee795f190cae5c36eaf2c86 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Thu, 14 Apr 2016 09:51:21 -0700 Subject: [PATCH 3/3] Improve comment explaining ContainsLexicalThisInComputedPropertyName --- src/compiler/binder.ts | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index 5ed1e9665d5..f92f6789d29 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -1911,9 +1911,14 @@ namespace ts { // Instead, we mark the container as ES6, so that it can properly handle the transform. transformFlags = TransformFlags.ContainsComputedPropertyName; if (subtreeFlags & TransformFlags.ContainsLexicalThis) { - // A computed method name that contains `this` needs to - // distinguish itself from the normal case of a method body containing `this`. - // So convert ContainsLexicalThis to ContainsLexicalThisInComputedPropertyName + // A computed method name like `[this.getName()](x: string) { ... }` needs to + // distinguish itself from the normal case of a method body containing `this`: + // `this` inside a method doesn't need to be rewritten (the method provides `this`), + // whereas `this` inside a computed name *might* need to be rewritten if the class/object + // is inside an arrow function: + // `_this = this; () => class K { [_this.getName()]() { ... } }` + // To make this distinction, use ContainsLexicalThisInComputedPropertyName + // instead of ContainsLexicalThis for computed property names transformFlags |= TransformFlags.ContainsLexicalThisInComputedPropertyName; } break; @@ -1952,6 +1957,8 @@ namespace ts { transformFlags = TransformFlags.AssertES6; } if (subtreeFlags & TransformFlags.ContainsLexicalThisInComputedPropertyName) { + // A computed property name containing `this` might need to be rewritten, + // so propagate the ContainsLexicalThis flag upward. transformFlags |= TransformFlags.ContainsLexicalThis; } break; @@ -2266,6 +2273,8 @@ namespace ts { transformFlags |= TransformFlags.AssertTypeScript; } if (subtreeFlags & TransformFlags.ContainsLexicalThisInComputedPropertyName) { + // A computed property name containing `this` might need to be rewritten, + // so propagate the ContainsLexicalThis flag upward. transformFlags |= TransformFlags.ContainsLexicalThis; } @@ -2285,6 +2294,8 @@ namespace ts { transformFlags |= TransformFlags.AssertTypeScript; } if (subtreeFlags & TransformFlags.ContainsLexicalThisInComputedPropertyName) { + // A computed property name containing `this` might need to be rewritten, + // so propagate the ContainsLexicalThis flag upward. transformFlags |= TransformFlags.ContainsLexicalThis; }