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.

Sign up for GitHub

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

Jump to bottom

Memoize class binding when compiling private methods and static elements #15701

Merged
merged 4 commits into from
Jul 19, 2023
Merged

Memoize class binding when compiling private methods and static elements #15701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f284adc
add test cases
JLHwung Jun 13, 2023
198653a
fix: memoise class binding when lowering static elements and private …
JLHwung Jun 13, 2023
344cc36
update test fixtures
JLHwung Jun 13, 2023
de6c345
update test fixtures
JLHwung Jul 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
148 changes: 94 additions & 54 deletions packages/babel-helper-create-class-features-plugin/src/fields.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,8 @@ type ReplaceThisState = {
innerBinding: t.Identifier | null;
};

type ReplaceInnerBindingReferenceState = ReplaceThisState;

const thisContextVisitor = traverse.visitors.merge<ReplaceThisState>([
{
UnaryExpression(path) {
Expand Down Expand Up @@ -984,7 +986,7 @@ const thisContextVisitor = traverse.visitors.merge<ReplaceThisState>([
environmentVisitor,
]);

const innerReferencesVisitor: Visitor<ReplaceThisState> = {
const innerReferencesVisitor: Visitor<ReplaceInnerBindingReferenceState> = {
ReferencedIdentifier(path, state) {
if (
path.scope.bindingIdentifierEquals(path.node.name, state.innerBinding)
Expand All @@ -998,42 +1000,23 @@ const innerReferencesVisitor: Visitor<ReplaceThisState> = {
function replaceThisContext(
path: PropPath,
ref: t.Identifier,
getSuperRef: () => t.Identifier,
file: File,
isStaticBlock: boolean,
constantSuper: boolean,
innerBindingRef: t.Identifier | null,
) {
const state: ReplaceThisState = {
classRef: ref,
needsClassRef: false,
innerBinding: innerBindingRef,
};

const replacer = new ReplaceSupers({
methodPath: path,
constantSuper,
file,
refToPreserve: ref,
getSuperRef,
getObjectRef() {
state.needsClassRef = true;
// @ts-expect-error: TS doesn't infer that path.node is not a StaticBlock
return t.isStaticBlock?.(path.node) || path.node.static
? ref
: t.memberExpression(ref, t.identifier("prototype"));
},
});
replacer.replace();
if (isStaticBlock || path.isProperty()) {
if (!path.isMethod()) {
// replace `this` in property initializers and static blocks
path.traverse(thisContextVisitor, state);
}

// todo: use innerBinding.referencePaths to avoid full traversal
if (
innerBindingRef != null &&
state.classRef?.name &&
state.classRef.name !== innerBindingRef?.name
state.classRef.name !== innerBindingRef.name
) {
path.traverse(innerReferencesVisitor, state);
}
Expand Down Expand Up @@ -1075,23 +1058,47 @@ function inheritPropComments<T extends t.Node>(node: T, prop: PropPath) {
return node;
}

/**
* ClassRefFlag records the requirement of the class binding reference.
*
* @enum {number}
*/
const enum ClassRefFlag {
None,
/**
* When this flag is enabled, the binding reference can be the class id,
* if exists, or the uid identifier generated for class expression. The
* reference is safe to be consumed by [[Define]].
*/
ForDefine = 1 << 0,
/**
* When this flag is enabled, the reference must be a uid, because the outer
* class binding can be mutated by user codes.
* E.g.
* class C { static p = C }; const oldC = C; C = null; oldC.p;
* we must memoize class `C` before defining the property `p`.
*/
ForInnerBinding = 1 << 1,
}

export function buildFieldsInitNodes(
ref: t.Identifier,
ref: t.Identifier | null,
superRef: t.Expression | undefined,
props: PropPath[],
privateNamesMap: PrivateNamesMap,
state: File,
file: File,
setPublicClassFields: boolean,
privateFieldsAsProperties: boolean,
constantSuper: boolean,
innerBindingRef: t.Identifier,
innerBindingRef: t.Identifier | null,
) {
let needsClassRef = false;
let classRefFlags = ClassRefFlag.None;
let injectSuperRef: t.Identifier;
const staticNodes: t.Statement[] = [];
const instanceNodes: t.Statement[] = [];
// These nodes are pure and can be moved to the closest statement position
const pureStaticNodes: t.FunctionDeclaration[] = [];
let classBindingNode: t.ExpressionStatement | null = null;

const getSuperRef = t.isIdentifier(superRef)
? () => superRef
Expand All @@ -1101,6 +1108,10 @@ export function buildFieldsInitNodes(
return injectSuperRef;
};

const classRefForInnerBinding =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When ref is not defined and classRefFlags.ForInnerBinding is not enabled, e.g. compiling a plain class class C {} without private methods / static elements, we will generate a uid identifier but we do not insert it to the final AST. In this case the memoizer variable _class0, _class1, etc. is not continuous.

ref ?? props[0].scope.generateUidIdentifier("class");
ref ??= t.cloneNode(innerBindingRef);

for (const prop of props) {
prop.isClassProperty() && ts.assertFieldTransformed(prop);

Expand All @@ -1113,17 +1124,36 @@ export function buildFieldsInitNodes(
const isMethod = !isField;
const isStaticBlock = prop.isStaticBlock?.();

if (isStatic) classRefFlags |= ClassRefFlag.ForDefine;

if (isStatic || (isMethod && isPrivate) || isStaticBlock) {
new ReplaceSupers({
methodPath: prop,
constantSuper,
file: file,
refToPreserve: innerBindingRef,
getSuperRef,
getObjectRef() {
classRefFlags |= ClassRefFlag.ForInnerBinding;
if (isStatic || isStaticBlock) {
return classRefForInnerBinding;
} else {
return t.memberExpression(
classRefForInnerBinding,
t.identifier("prototype"),
);
}
},
}).replace();

const replaced = replaceThisContext(
prop,
ref,
getSuperRef,
state,
isStaticBlock,
constantSuper,
classRefForInnerBinding,
innerBindingRef,
);
needsClassRef = needsClassRef || replaced;
if (replaced) {
classRefFlags |= ClassRefFlag.ForInnerBinding;
}
}

// TODO(ts): there are so many `ts-expect-error` inside cases since
Expand All @@ -1149,14 +1179,12 @@ export function buildFieldsInitNodes(
break;
}
case isStatic && isPrivate && isField && privateFieldsAsProperties:
needsClassRef = true;
staticNodes.push(
// @ts-expect-error checked in switch
buildPrivateFieldInitLoose(t.cloneNode(ref), prop, privateNamesMap),
);
break;
case isStatic && isPrivate && isField && !privateFieldsAsProperties:
needsClassRef = true;
staticNodes.push(
// @ts-expect-error checked in switch
buildPrivateStaticFieldInitSpec(prop, privateNamesMap),
Expand All @@ -1170,17 +1198,15 @@ export function buildFieldsInitNodes(
// not going to happen.
// @ts-expect-error checked in switch
if (!isNameOrLength(prop.node)) {
needsClassRef = true;
// @ts-expect-error checked in switch
staticNodes.push(buildPublicFieldInitLoose(t.cloneNode(ref), prop));
break;
}
// falls through
case isStatic && isPublic && isField && !setPublicClassFields:
needsClassRef = true;
staticNodes.push(
// @ts-expect-error checked in switch
buildPublicFieldInitSpec(t.cloneNode(ref), prop, state),
buildPublicFieldInitSpec(t.cloneNode(ref), prop, file),
);
break;
case isInstance && isPrivate && isField && privateFieldsAsProperties:
Expand All @@ -1196,7 +1222,7 @@ export function buildFieldsInitNodes(
// @ts-expect-error checked in switch
prop,
privateNamesMap,
state,
file,
),
);
break;
Expand Down Expand Up @@ -1225,7 +1251,7 @@ export function buildFieldsInitNodes(
// @ts-expect-error checked in switch
prop,
privateNamesMap,
state,
file,
),
);
pureStaticNodes.push(
Expand All @@ -1238,7 +1264,6 @@ export function buildFieldsInitNodes(
);
break;
case isStatic && isPrivate && isMethod && !privateFieldsAsProperties:
needsClassRef = true;
staticNodes.unshift(
// @ts-expect-error checked in switch
buildPrivateStaticFieldInitSpec(prop, privateNamesMap),
Expand All @@ -1253,13 +1278,12 @@ export function buildFieldsInitNodes(
);
break;
case isStatic && isPrivate && isMethod && privateFieldsAsProperties:
needsClassRef = true;
staticNodes.unshift(
buildPrivateStaticMethodInitLoose(
t.cloneNode(ref),
// @ts-expect-error checked in switch
prop,
state,
file,
privateNamesMap,
),
);
Expand All @@ -1279,18 +1303,29 @@ export function buildFieldsInitNodes(
case isInstance && isPublic && isField && !setPublicClassFields:
instanceNodes.push(
// @ts-expect-error checked in switch
buildPublicFieldInitSpec(t.thisExpression(), prop, state),
buildPublicFieldInitSpec(t.thisExpression(), prop, file),
);
break;
default:
throw new Error("Unreachable.");
}
}

if (classRefFlags & ClassRefFlag.ForInnerBinding && innerBindingRef != null) {
classBindingNode = t.expressionStatement(
t.assignmentExpression(
"=",
t.cloneNode(classRefForInnerBinding),
t.cloneNode(innerBindingRef),
),
);
}

return {
staticNodes: staticNodes.filter(Boolean),
instanceNodes: instanceNodes.filter(Boolean),
pureStaticNodes: pureStaticNodes.filter(Boolean),
classBindingNode,
wrapClass(path: NodePath<t.Class>) {
for (const prop of props) {
// Delete leading comments so that they don't get attached as
Expand All @@ -1310,16 +1345,21 @@ export function buildFieldsInitNodes(
);
}

if (!needsClassRef) return path;

if (path.isClassExpression()) {
path.scope.push({ id: ref });
path.replaceWith(
t.assignmentExpression("=", t.cloneNode(ref), path.node),
);
} else if (!path.node.id) {
// Anonymous class declaration
path.node.id = ref;
if (classRefFlags !== ClassRefFlag.None) {
if (path.isClassExpression()) {
path.scope.push({ id: ref });
path.replaceWith(
t.assignmentExpression("=", t.cloneNode(ref), path.node),
);
} else {
if (innerBindingRef == null) {
// export anonymous class declaration
path.node.id = ref;
}
if (classBindingNode != null) {
path.scope.push({ id: classRefForInnerBinding });
}
}
}

return path;
Expand Down