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

refactor(compiler): use updateConstructor helper in native-constructor.ts #4741

Merged
merged 1 commit into from
Sep 7, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ const updateNativeHostComponentMembers = (
): ts.ClassElement[] => {
const classMembers = removeStaticMetaProperties(classNode);

updateNativeConstructor(classMembers, moduleFile, cmp);
updateNativeConstructor(classMembers, moduleFile, cmp, classNode);
addNativeConnectedCallback(classMembers, cmp);
addNativeElementGetter(classMembers, cmp);
addWatchers(classMembers, cmp);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import ts from 'typescript';

import type * as d from '../../../declarations';
import { addCreateEvents } from '../create-event';
import { retrieveTsModifiers } from '../transform-utils';
import { updateConstructor } from '../transform-utils';

/**
* Updates a constructor to include:
Expand All @@ -16,47 +16,21 @@ import { retrieveTsModifiers } from '../transform-utils';
* @param classMembers the class elements to modify
* @param moduleFile the Stencil module representation of the component class
* @param cmp the component metadata generated for the component
* @param classNode the TypeScript syntax tree node for the class
*/
export const updateNativeConstructor = (
classMembers: ts.ClassElement[],
moduleFile: d.Module,
cmp: d.ComponentCompilerMeta,
classNode: ts.ClassDeclaration,
): void => {
if (cmp.isPlain) {
return;
}
const cstrMethodIndex = classMembers.findIndex((m) => m.kind === ts.SyntaxKind.Constructor);

if (cstrMethodIndex >= 0) {
// add to the existing constructor()
const cstrMethod = classMembers[cstrMethodIndex] as ts.ConstructorDeclaration;
// a constructor may not have a body (e.g. in the case of constructor overloads)
const cstrBodyStatements: ts.NodeArray<ts.Statement> = cstrMethod.body?.statements ?? ts.factory.createNodeArray();
const nativeCstrStatements = [...nativeInit(cmp), ...addCreateEvents(moduleFile, cmp)];

let statements: ts.Statement[] = [...nativeInit(cmp), ...addCreateEvents(moduleFile, cmp), ...cstrBodyStatements];

const hasSuper = cstrBodyStatements.some((s) => s.kind === ts.SyntaxKind.SuperKeyword);
if (!hasSuper) {
statements = [createNativeConstructorSuper(), ...statements];
}

classMembers[cstrMethodIndex] = ts.factory.updateConstructorDeclaration(
cstrMethod,
retrieveTsModifiers(cstrMethod),
cstrMethod.parameters,
ts.factory.updateBlock(cstrMethod.body, statements),
);
} else {
// create a constructor()
const statements: ts.Statement[] = [
createNativeConstructorSuper(),
...nativeInit(cmp),
...addCreateEvents(moduleFile, cmp),
];

const cstrMethod = ts.factory.createConstructorDeclaration(undefined, [], ts.factory.createBlock(statements, true));
classMembers.unshift(cstrMethod);
}
updateConstructor(classNode, classMembers, nativeCstrStatements);
};

/**
Expand Down Expand Up @@ -102,13 +76,3 @@ const nativeAttachShadowStatement = (): ts.ExpressionStatement => {
),
);
};

/**
* Create an expression statement for calling `super()` for a class.
* @returns the generated expression statement
*/
const createNativeConstructorSuper = (): ts.ExpressionStatement => {
return ts.factory.createExpressionStatement(
ts.factory.createCallExpression(ts.factory.createSuper(), undefined, undefined),
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { augmentDiagnosticWithNode, buildError } from '@utils';
import ts from 'typescript';

import type * as d from '../../../declarations';
import { retrieveTsDecorators, retrieveTsModifiers } from '../transform-utils';
import { retrieveTsDecorators, retrieveTsModifiers, updateConstructor } from '../transform-utils';
import { componentDecoratorToStatic } from './component-decorator';
import { isDecoratorNamed } from './decorator-utils';
import {
Expand Down Expand Up @@ -410,99 +410,6 @@ function handleClassFields(classNode: ts.ClassDeclaration, classMembers: ts.Clas
}
}

/**
* Helper util for updating the constructor on a class declaration AST node.
*
* @param classNode the class node whose constructor will be updated
* @param classMembers a list of class members for that class
* @param statements a list of statements which should be added to the
* constructor
* @returns a list of updated class elements
*/
export const updateConstructor = (
classNode: ts.ClassDeclaration,
classMembers: ts.ClassElement[],
statements: ts.Statement[],
): ts.ClassElement[] => {
const constructorIndex = classMembers.findIndex((m) => m.kind === ts.SyntaxKind.Constructor);
const constructorMethod = classMembers[constructorIndex];

if (constructorIndex >= 0 && ts.isConstructorDeclaration(constructorMethod)) {
const constructorBodyStatements: ts.NodeArray<ts.Statement> =
constructorMethod.body?.statements ?? ts.factory.createNodeArray();
const hasSuper = constructorBodyStatements.some((s) => s.kind === ts.SyntaxKind.SuperKeyword);

if (!hasSuper && needsSuper(classNode)) {
// if there is no super and it needs one the statements comprising the
// body of the constructor should be:
//
// 1. the `super()` call
// 2. the new statements we've created to initialize fields
// 3. the statements currently comprising the body of the constructor
statements = [createConstructorBodyWithSuper(), ...statements, ...constructorBodyStatements];
} else {
// if no super is needed then the body of the constructor should be:
//
// 1. the new statements we've created to initialize fields
// 2. the statements currently comprising the body of the constructor
statements = [...statements, ...constructorBodyStatements];
}

classMembers[constructorIndex] = ts.factory.updateConstructorDeclaration(
constructorMethod,
retrieveTsModifiers(constructorMethod),
constructorMethod.parameters,
ts.factory.updateBlock(constructorMethod?.body ?? ts.factory.createBlock([]), statements),
);
} else {
// we don't seem to have a constructor, so let's create one and stick it
// into the array of class elements
if (needsSuper(classNode)) {
statements = [createConstructorBodyWithSuper(), ...statements];
}

classMembers = [
ts.factory.createConstructorDeclaration(undefined, [], ts.factory.createBlock(statements, true)),
...classMembers,
];
}

return classMembers;
};

/**
* Check that a given class declaration should have a `super()` call in its
* constructor. This is something we can check by looking for a
* {@link ts.HeritageClause} on the class's AST node.
*
* @param classDeclaration a class declaration AST node
* @returns whether this class has parents or not
*/
const needsSuper = (classDeclaration: ts.ClassDeclaration): boolean => {
const hasHeritageClauses = classDeclaration.heritageClauses && classDeclaration.heritageClauses.length > 0;

if (hasHeritageClauses) {
// A {@link ts.SyntaxKind.HeritageClause} node may be for extending a
// superclass _or_ for implementing an interface. We only want to add a
// `super()` call to our synthetic constructor here in the case that there
// is a superclass, so we can check for that situation by checking for the
// presence of a heritage clause with the `.token` property set to
// `ts.SyntaxKind.ExtendsKeyword`.
return classDeclaration.heritageClauses.some((clause) => clause.token === ts.SyntaxKind.ExtendsKeyword);
}
return false;
};

/**
* Create a statement with a call to `super()` suitable for including in the body of a constructor.
* @returns a {@link ts.ExpressionStatement} node equivalent to `super()`
*/
const createConstructorBodyWithSuper = (): ts.ExpressionStatement => {
return ts.factory.createExpressionStatement(
ts.factory.createCallExpression(ts.factory.createIdentifier('super'), undefined, undefined),
);
};

/**
* Check whether a given class element should be rewritten from a class field
* to a constructor-initialized value. This is basically the case for fields
Expand Down
94 changes: 94 additions & 0 deletions src/compiler/transformers/transform-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -945,3 +945,97 @@ export const retrieveTsDecorators = (node: ts.Node): ReadonlyArray<ts.Decorator>
export const retrieveTsModifiers = (node: ts.Node): ReadonlyArray<ts.Modifier> | undefined => {
return ts.canHaveModifiers(node) ? ts.getModifiers(node) : undefined;
};

/**
* Helper util for updating the constructor on a class declaration AST node.
*
* @param classNode the class node whose constructor will be updated
* @param classMembers a list of class members for that class
* @param statements a list of statements which should be added to the
* constructor
* @returns a list of updated class elements
*/
export const updateConstructor = (
classNode: ts.ClassDeclaration,
classMembers: ts.ClassElement[],
statements: ts.Statement[],
): ts.ClassElement[] => {
const constructorIndex = classMembers.findIndex((m) => m.kind === ts.SyntaxKind.Constructor);
const constructorMethod = classMembers[constructorIndex];

if (constructorIndex >= 0 && ts.isConstructorDeclaration(constructorMethod)) {
const constructorBodyStatements: ts.NodeArray<ts.Statement> =
constructorMethod.body?.statements ?? ts.factory.createNodeArray();
const hasSuper = constructorBodyStatements.some((s) => s.kind === ts.SyntaxKind.SuperKeyword);

if (!hasSuper && needsSuper(classNode)) {
// if there is no super and it needs one the statements comprising the
// body of the constructor should be:
//
// 1. the `super()` call
// 2. the new statements we've created to initialize fields
// 3. the statements currently comprising the body of the constructor
statements = [createConstructorBodyWithSuper(), ...statements, ...constructorBodyStatements];
} else {
// if no super is needed then the body of the constructor should be:
//
// 1. the new statements we've created
// 2. the statements currently comprising the body of the constructor
statements = [...statements, ...constructorBodyStatements];
}

classMembers[constructorIndex] = ts.factory.updateConstructorDeclaration(
constructorMethod,
retrieveTsModifiers(constructorMethod),
constructorMethod.parameters,
ts.factory.updateBlock(constructorMethod?.body ?? ts.factory.createBlock([]), statements),
);
} else {
// we don't seem to have a constructor, so let's create one and stick it
// into the array of class elements
if (needsSuper(classNode)) {
statements = [createConstructorBodyWithSuper(), ...statements];
}

// add the new constructor to the class members, putting it at the
// beginning
classMembers.unshift(
ts.factory.createConstructorDeclaration(undefined, [], ts.factory.createBlock(statements, true)),
);
Comment on lines +1000 to +1004
Copy link
Contributor

Choose a reason for hiding this comment

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

Verifying my understanding - in moving this function from convert-decorators to here, is this the only block of code (other than comments) that changed?

Comparing this against - https://github.com/ionic-team/stencil/pull/4741/files#diff-84d52fb230ffdf800b6b3debb4f554136efd13683b396c03d1dd8a129f81a602L464-L468

-    classMembers = [
-      ts.factory.createConstructorDeclaration(undefined, [], ts.factory.createBlock(statements, true)),
-      ...classMembers,
-    ];
+    classMembers.unshift(
+     ts.factory.createConstructorDeclaration(undefined, [], ts.factory.createBlock(statements, true)),
+    );

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep! I made that change in order to ensure that the classMembers array that's passed in gets modified, since that aligns with how the modification of the constructor for the native constructor transformers was being done

}

return classMembers;
};

/**
* Check that a given class declaration should have a `super()` call in its
* constructor. This is something we can check by looking for a
* {@link ts.HeritageClause} on the class's AST node.
*
* @param classDeclaration a class declaration AST node
* @returns whether this class has parents or not
*/
const needsSuper = (classDeclaration: ts.ClassDeclaration): boolean => {
const hasHeritageClauses = classDeclaration.heritageClauses && classDeclaration.heritageClauses.length > 0;

if (hasHeritageClauses) {
// A {@link ts.SyntaxKind.HeritageClause} node may be for extending a
// superclass _or_ for implementing an interface. We only want to add a
// `super()` call to our synthetic constructor here in the case that there
// is a superclass, so we can check for that situation by checking for the
// presence of a heritage clause with the `.token` property set to
// `ts.SyntaxKind.ExtendsKeyword`.
return classDeclaration.heritageClauses.some((clause) => clause.token === ts.SyntaxKind.ExtendsKeyword);
}
return false;
};

/**
* Create a statement with a call to `super()` suitable for including in the body of a constructor.
* @returns a {@link ts.ExpressionStatement} node equivalent to `super()`
*/
const createConstructorBodyWithSuper = (): ts.ExpressionStatement => {
return ts.factory.createExpressionStatement(
ts.factory.createCallExpression(ts.factory.createIdentifier('super'), undefined, undefined),
);
};