Skip to content

Use 'static {}' for static fields when available and useDefineForClassFields is false #47707

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 1 commit into from
Feb 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 1 addition & 26 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28417,29 +28417,6 @@ namespace ts {
grammarErrorOnNode(right, Diagnostics.Cannot_assign_to_private_method_0_Private_methods_are_not_writable, idText(right));
}

if (lexicallyScopedSymbol?.valueDeclaration && (getEmitScriptTarget(compilerOptions) === ScriptTarget.ESNext && !useDefineForClassFields)) {
const lexicalClass = getContainingClass(lexicallyScopedSymbol.valueDeclaration);
const parentStaticFieldInitializer = findAncestor(node, (n) => {
if (n === lexicalClass) return "quit";
if (isPropertyDeclaration(n.parent) && hasStaticModifier(n.parent) && n.parent.initializer === n && n.parent.parent === lexicalClass) {
return true;
}
return false;
});
if (parentStaticFieldInitializer) {
const parentStaticFieldInitializerSymbol = getSymbolOfNode(parentStaticFieldInitializer.parent);
Debug.assert(parentStaticFieldInitializerSymbol, "Initializer without declaration symbol");
const diagnostic = error(node,
Diagnostics.Property_0_may_not_be_used_in_a_static_property_s_initializer_in_the_same_class_when_target_is_esnext_and_useDefineForClassFields_is_false,
symbolName(lexicallyScopedSymbol));
addRelatedInfo(diagnostic,
createDiagnosticForNode(parentStaticFieldInitializer.parent,
Diagnostics.Initializer_for_property_0,
symbolName(parentStaticFieldInitializerSymbol))
);
}
}

if (isAnyLike) {
if (lexicallyScopedSymbol) {
return isErrorType(apparentType) ? errorType : apparentType;
Expand Down Expand Up @@ -34645,9 +34622,7 @@ namespace ts {
checkVariableLikeDeclaration(node);

setNodeLinksForPrivateIdentifierScope(node);
if (isPrivateIdentifier(node.name) && hasStaticModifier(node) && node.initializer && languageVersion === ScriptTarget.ESNext && !compilerOptions.useDefineForClassFields) {
error(node.initializer, Diagnostics.Static_fields_with_private_names_can_t_have_initializers_when_the_useDefineForClassFields_flag_is_not_specified_with_a_target_of_esnext_Consider_adding_the_useDefineForClassFields_flag);
}

// property signatures already report "initializer not allowed in ambient context" elsewhere
if (hasSyntacticModifier(node, ModifierFlags.Abstract) && node.kind === SyntaxKind.PropertyDeclaration && node.initializer) {
error(node, Diagnostics.Property_0_cannot_have_an_initializer_because_it_is_marked_abstract, declarationNameToString(node.name));
Expand Down
8 changes: 0 additions & 8 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3281,10 +3281,6 @@
"category": "Error",
"code": 2804
},
"Static fields with private names can't have initializers when the '--useDefineForClassFields' flag is not specified with a '--target' of 'esnext'. Consider adding the '--useDefineForClassFields' flag.": {
"category": "Error",
"code": 2805
},
"Private accessor was defined without a getter.": {
"category": "Error",
"code": 2806
Expand All @@ -3301,10 +3297,6 @@
"category": "Error",
"code": 2809
},
"Property '{0}' may not be used in a static property's initializer in the same class when 'target' is 'esnext' and 'useDefineForClassFields' is 'false'.": {
"category": "Error",
"code": 2810
},
"Initializer for property '{0}'": {
"category": "Error",
"code": 2811
Expand Down
98 changes: 78 additions & 20 deletions src/compiler/transformers/classFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,13 @@ namespace ts {

const shouldTransformPrivateElementsOrClassStaticBlocks = languageVersion < ScriptTarget.ES2022;

// We need to transform `this` in a static initializer into a reference to the class
// when targeting < ES2022 since the assignment will be moved outside of the class body.
const shouldTransformThisInStaticInitializers = languageVersion < ScriptTarget.ES2022;

// We don't need to transform `super` property access when targeting ES5, ES3 because
// the es2015 transformation handles those.
const shouldTransformSuperInStaticInitializers = (languageVersion <= ScriptTarget.ES2021 || !useDefineForClassFields) && languageVersion >= ScriptTarget.ES2015;
const shouldTransformThisInStaticInitializers = languageVersion <= ScriptTarget.ES2021 || !useDefineForClassFields;
const shouldTransformSuperInStaticInitializers = shouldTransformThisInStaticInitializers && languageVersion >= ScriptTarget.ES2015;

const previousOnSubstituteNode = context.onSubstituteNode;
context.onSubstituteNode = onSubstituteNode;
Expand Down Expand Up @@ -422,6 +425,11 @@ namespace ts {

if (isPrivateIdentifier(node.name)) {
if (!shouldTransformPrivateElementsOrClassStaticBlocks) {
if (isStatic(node)) {
// static fields are left as is
return visitEachChild(node, visitor, context);
}

// Initializer is elided as the field is initialized in transformConstructor.
return factory.updatePropertyDeclaration(
node,
Expand All @@ -448,6 +456,28 @@ namespace ts {
if (expr && !isSimpleInlineableExpression(expr)) {
getPendingExpressions().push(expr);
}

if (isStatic(node) && !shouldTransformPrivateElementsOrClassStaticBlocks && !useDefineForClassFields) {
const initializerStatement = transformPropertyOrClassStaticBlock(node, factory.createThis());
if (initializerStatement) {
const staticBlock = factory.createClassStaticBlockDeclaration(
/*decorators*/ undefined,
/*modifiers*/ undefined,
factory.createBlock([initializerStatement])
);

setOriginalNode(staticBlock, node);
setCommentRange(staticBlock, node);

// Set the comment range for the statement to an empty synthetic range
// and drop synthetic comments from the statement to avoid printing them twice.
setCommentRange(initializerStatement, { pos: -1, end: -1 });
setSyntheticLeadingComments(initializerStatement, undefined);
setSyntheticTrailingComments(initializerStatement, undefined);
return staticBlock;
}
}

return undefined;
}

Expand Down Expand Up @@ -1006,8 +1036,6 @@ namespace ts {
enableSubstitutionForClassStaticThisOrSuperReference();
}

const staticProperties = getStaticPropertiesAndClassStaticBlock(node);

// If a class has private static fields, or a static field has a `this` or `super` reference,
// then we need to allocate a temp variable to hold on to that reference.
let pendingClassReferenceAssignment: BinaryExpression | undefined;
Expand Down Expand Up @@ -1047,6 +1075,7 @@ namespace ts {
// HasLexicalDeclaration (N) : Determines if the argument identifier has a binding in this environment record that was created using
// a lexical declaration such as a LexicalDeclaration or a ClassDeclaration.

const staticProperties = getStaticPropertiesAndClassStaticBlock(node);
if (some(staticProperties)) {
addPropertyOrClassStaticBlockStatements(statements, staticProperties, factory.getInternalName(node));
}
Expand Down Expand Up @@ -1102,7 +1131,7 @@ namespace ts {
transformClassMembers(node, isDerivedClass)
);

const hasTransformableStatics = some(staticPropertiesOrClassStaticBlocks, p => isClassStaticBlockDeclaration(p) || !!p.initializer || (shouldTransformPrivateElementsOrClassStaticBlocks && isPrivateIdentifier(p.name)));
const hasTransformableStatics = shouldTransformPrivateElementsOrClassStaticBlocks && some(staticPropertiesOrClassStaticBlocks, p => isClassStaticBlockDeclaration(p) || !!p.initializer || isPrivateIdentifier(p.name));
if (hasTransformableStatics || some(pendingExpressions)) {
if (isDecoratedClassDeclaration) {
Debug.assertIsDefined(pendingStatements, "Decorated classes transformed by TypeScript are expected to be within a variable declaration.");
Expand Down Expand Up @@ -1156,6 +1185,7 @@ namespace ts {
}

function transformClassMembers(node: ClassDeclaration | ClassExpression, isDerivedClass: boolean) {
const members: ClassElement[] = [];
if (shouldTransformPrivateElementsOrClassStaticBlocks) {
// Declare private names.
for (const member of node.members) {
Expand All @@ -1169,12 +1199,26 @@ namespace ts {
}
}

const members: ClassElement[] = [];
const constructor = transformConstructor(node, isDerivedClass);
const visitedMembers = visitNodes(node.members, classElementVisitor, isClassElement);

if (constructor) {
members.push(constructor);
}
addRange(members, visitNodes(node.members, classElementVisitor, isClassElement));

if (!shouldTransformPrivateElementsOrClassStaticBlocks && some(pendingExpressions)) {
members.push(factory.createClassStaticBlockDeclaration(
/*decorators*/ undefined,
/*modifiers*/ undefined,
factory.createBlock([
factory.createExpressionStatement(factory.inlineExpressions(pendingExpressions))
])
));
pendingExpressions = undefined;
}

addRange(members, visitedMembers);

return setTextRange(factory.createNodeArray(members), /*location*/ node.members);
}

Expand Down Expand Up @@ -1374,27 +1418,41 @@ namespace ts {
*/
function addPropertyOrClassStaticBlockStatements(statements: Statement[], properties: readonly (PropertyDeclaration | ClassStaticBlockDeclaration)[], receiver: LeftHandSideExpression) {
for (const property of properties) {
const expression = isClassStaticBlockDeclaration(property) ?
transformClassStaticBlockDeclaration(property) :
transformProperty(property, receiver);
if (!expression) {
if (isStatic(property) && !shouldTransformPrivateElementsOrClassStaticBlocks && !useDefineForClassFields) {
continue;
}
const statement = factory.createExpressionStatement(expression);
setSourceMapRange(statement, moveRangePastModifiers(property));
setCommentRange(statement, property);
setOriginalNode(statement, property);

// `setOriginalNode` *copies* the `emitNode` from `property`, so now both
// `statement` and `expression` have a copy of the synthesized comments.
// Drop the comments from expression to avoid printing them twice.
setSyntheticLeadingComments(expression, undefined);
setSyntheticTrailingComments(expression, undefined);
const statement = transformPropertyOrClassStaticBlock(property, receiver);
if (!statement) {
continue;
}

statements.push(statement);
}
}

function transformPropertyOrClassStaticBlock(property: PropertyDeclaration | ClassStaticBlockDeclaration, receiver: LeftHandSideExpression) {
const expression = isClassStaticBlockDeclaration(property) ?
transformClassStaticBlockDeclaration(property) :
transformProperty(property, receiver);
if (!expression) {
return undefined;
}

const statement = factory.createExpressionStatement(expression);
setSourceMapRange(statement, moveRangePastModifiers(property));
setCommentRange(statement, property);
setOriginalNode(statement, property);

// `setOriginalNode` *copies* the `emitNode` from `property`, so now both
// `statement` and `expression` have a copy of the synthesized comments.
// Drop the comments from expression to avoid printing them twice.
setSyntheticLeadingComments(expression, undefined);
setSyntheticTrailingComments(expression, undefined);

return statement;
}

/**
* Generates assignment expressions for property initializers.
*
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/computedPropertyName.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ class D {
constructor() {
this[_a] = 0; // Error
}
static { _a = onInit; }
}
_a = onInit;
class E {
[onInit]() { } // Error
}
Expand Down
Loading