Add support for 'new.target' meta-property#12783
Conversation
src/compiler/core.ts
Outdated
| * @param value The value to append to the array. If `value` is `undefined`, nothing is | ||
| * appended. | ||
| * @param copyOnWrite Indicates whether to return a fresh array rather than modify the | ||
| * existing array. |
There was a problem hiding this comment.
Looks like this can be removed.
src/compiler/utilities.ts
Outdated
| return false; | ||
| } | ||
|
|
||
| export function getAllLabeledStatements(node: LabeledStatement): { statement: Statement; labeledStatements: LabeledStatement[]; } { |
src/compiler/transformers/es2015.ts
Outdated
|
|
||
| // Subtree facts | ||
| NewTarget = 1 << 14, // Contains a 'new.target' meta-property | ||
| NewTargetInComputedPropertyName = 1 << 15, // Contains a 'new.target' meta-property in a computed property name. |
There was a problem hiding this comment.
Why is it important that this be distinct from NewTarget?
There was a problem hiding this comment.
When we compute flags for a member, such as a method or accessor, it both has its own lexical scope (for the parameters and body) as well as shares the lexical scope of its container (due to the computed property name). Without this flag, encountering a NewTarget in a computed property name would incorrectly indicate that NewTarget was set in the body of the method, not in the container.
|
I'll have to dive in, but can you explain the es2015 transform changes you needed to make? |
|
Regarding the changes to the transformer, I am trying to clean up the es2015 transformer and remove some of the variables we use to track state as we descend the tree, this should reduce the overall memory footprint due to values saved on the stack. Ideally I can use this same mechanism to eventually remove some I can isolate these changes to a separate PR if it would be more clear. |
| break; | ||
| } | ||
|
|
||
| const captureNewTargetStatement = createVariableStatement( |
There was a problem hiding this comment.
newTarget could be undefined if you're somehow not handling a case - consider adding a
Debug.assert(!!newTarget, "'newTarget' should be defined for " + (ts.SyntaxKind as any)[node.kind])or something similar.
src/compiler/transformers/es2015.ts
Outdated
| return node; | ||
| if (convertedLoopState) { | ||
| if (hierarchyFacts & HierarchyFacts.ArrowFunction) { | ||
| // if the enclosing function is an ArrowFunction is then we use the captured 'this' keyword. |
There was a problem hiding this comment.
"is then" -> ", then"
| return false; | ||
| } | ||
|
|
||
| export function unwrapInnermostStatmentOfLabel(node: LabeledStatement, beforeUnwrapLabelCallback?: (node: LabeledStatement) => void) { |
There was a problem hiding this comment.
You only use this function once - do we anticipate using it again?
There was a problem hiding this comment.
Yes, I also am using this in the asyncGenerators branch
src/compiler/utilities.ts
Outdated
| || kind === SyntaxKind.TrueKeyword | ||
| || kind === SyntaxKind.SuperKeyword | ||
| || kind === SyntaxKind.NonNullExpression | ||
| || kind === SyntaxKind.MetaProperty |
There was a problem hiding this comment.
Do we want to just make this a switch? I think it's asking for it at this point.
|
I get the crux of the changes which look alright to me - I'm not an expert at the loop conversion code, so @vladima may be a good person to check in with. |
This adds support for the
new.targetmeta-property introduced in ES6. This also adds best-effort down-level emit for ES5/3.NewTarget meta-property
The
new.targetmeta-property is new syntax introduced in ES6. When you create an instance of a constructor vianew, the value ofnew.targetis set to be a reference to the constructor function initially used to allocate the instance. If a function is called rather than constructed vianew,new.targetis set toundefined. In addition,new.targetis lexically scoped to the body of the function. As such, it can be captured inside an arrow function.While it is legal to use
new.targetin methods and accessors in ES6, those members cannot be called vianewas they are not constructors. As such, we have chosen to make it an error to referencenew.targetin the body of a method or accessor.ES5 emit
new.targetis a pure ES6 syntactic construct that can only be emitted down-level on a best-effort basis. While our emit fornew.targetis sufficient for ES5 code that interacts with other ES5 code, it does not fully emulatenew.targetin an ES6 host environment. What this means is that ournew.targetemit is not compatible withReflect.constructas we have no way to intercept this API. This emit also is intolerant of explicit changes to theconstructorproperty of the constructor's prototype.That said, the reason we are introducing best-effort down-level emit for
new.targetare for several specific use cases pairing ES5 emit for classes withObject.setPrototypeOfor__proto__. One such use case is inheriting fromErrorin NodeJS v4 and higher:Also, while we can use
this.constructorto emulatenew.targetfor classes, we must be more defensive fornew.targetin a function declaration or function expression. The reason for this is that calling an imported function in a CommonJS, AMD, or System module results in thethisargument of the function being bound to the module object (i.e.func()becomespackage_1.func()). Also, function expressions are often assigned to a property of an object which could in turn become thethisif the function is called as a member.To defend against this case, the down-level emit for
new.targetin functions is the following:Fixes #2551
Related: