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

Simplify class fields injetion after super() #16123

Merged
merged 2 commits into from
Nov 25, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,11 @@ export function buildDecoratedClass(
}

return {
instanceNodes: [template.statement.ast`${t.cloneNode(initializeId)}(this)`],
instanceNodes: [
template.statement.ast`
${t.cloneNode(initializeId)}(this)
` as t.ExpressionStatement,
],
wrapClass(path: NodePath<t.Class>) {
path.replaceWith(replacement);
return path.get(classPathDesc) as NodePath;
Expand Down
25 changes: 15 additions & 10 deletions packages/babel-helper-create-class-features-plugin/src/fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,7 @@ function buildPrivateFieldInitLoose(
writable: true,
value: ${value}
});
`,
` as t.ExpressionStatement,
prop,
);
}
Expand All @@ -607,7 +607,7 @@ function buildPrivateInstanceFieldInitSpec(
// enumerable is always false for private elements
writable: true,
value: ${value},
})`,
})` as t.ExpressionStatement,
prop,
);
}
Expand All @@ -622,7 +622,7 @@ function buildPrivateInstanceFieldInitSpec(
writable: true,
value: ${value}
},
)`,
)` as t.ExpressionStatement,
prop,
);
}
Expand Down Expand Up @@ -689,7 +689,7 @@ function buildPrivateMethodInitLoose(
// writable is false by default
value: ${methodId.name}
});
`,
` as t.ExpressionStatement,
prop,
);
}
Expand All @@ -709,7 +709,7 @@ function buildPrivateMethodInitLoose(
get: ${getId ? getId.name : prop.scope.buildUndefinedNode()},
set: ${setId ? setId.name : prop.scope.buildUndefinedNode()}
});
`,
` as t.ExpressionStatement,
prop,
);
}
Expand Down Expand Up @@ -766,7 +766,7 @@ function buildPrivateAccessorInitialization(
get: ${getId ? getId.name : prop.scope.buildUndefinedNode()},
set: ${setId ? setId.name : prop.scope.buildUndefinedNode()}
});
`,
` as t.ExpressionStatement,
prop,
);
}
Expand All @@ -781,7 +781,7 @@ function buildPrivateAccessorInitialization(
get: ${getId ? getId.name : prop.scope.buildUndefinedNode()},
set: ${setId ? setId.name : prop.scope.buildUndefinedNode()}
},
)`,
)` as t.ExpressionStatement,
prop,
);
}
Expand All @@ -798,7 +798,7 @@ function buildPrivateInstanceMethodInitialization(
if (!process.env.BABEL_8_BREAKING) {
if (!state.availableHelper("classPrivateMethodInitSpec")) {
return inheritPropComments(
template.statement.ast`${id}.add(${ref})`,
template.statement.ast`${id}.add(${ref})` as t.ExpressionStatement,
prop,
);
}
Expand All @@ -809,7 +809,7 @@ function buildPrivateInstanceMethodInitialization(
template.statement.ast`${helper}(
${t.thisExpression()},
${t.cloneNode(id)}
)`,
)` as t.ExpressionStatement,
prop,
);
}
Expand Down Expand Up @@ -1095,7 +1095,8 @@ export function buildFieldsInitNodes(
let classRefFlags = ClassRefFlag.None;
let injectSuperRef: t.Identifier;
const staticNodes: t.Statement[] = [];
const instanceNodes: t.Statement[] = [];
const instanceNodes: t.ExpressionStatement[] = [];
let lastInstanceNodeReturnsThis = false;
// These nodes are pure and can be moved to the closest statement position
const pureStaticNodes: t.FunctionDeclaration[] = [];
let classBindingNode: t.ExpressionStatement | null = null;
Expand Down Expand Up @@ -1156,6 +1157,8 @@ export function buildFieldsInitNodes(
}
}

lastInstanceNodeReturnsThis = false;

// TODO(ts): there are so many `ts-expect-error` inside cases since
// ts can not infer type from pre-computed values (or a case test)
// even change `isStaticBlock` to `t.isStaticBlock(prop)` will not make prop
Expand Down Expand Up @@ -1301,6 +1304,7 @@ export function buildFieldsInitNodes(
instanceNodes.push(buildPublicFieldInitLoose(t.thisExpression(), prop));
break;
case isInstance && isPublic && isField && !setPublicClassFields:
lastInstanceNodeReturnsThis = true;
instanceNodes.push(
// @ts-expect-error checked in switch
buildPublicFieldInitSpec(t.thisExpression(), prop, file),
Expand All @@ -1324,6 +1328,7 @@ export function buildFieldsInitNodes(
return {
staticNodes: staticNodes.filter(Boolean),
instanceNodes: instanceNodes.filter(Boolean),
lastInstanceNodeReturnsThis,
pureStaticNodes: pureStaticNodes.filter(Boolean),
classBindingNode,
wrapClass(path: NodePath<t.Class>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,8 @@ export function createClassFeaturePlugin({

let keysNodes: t.Statement[],
staticNodes: t.Statement[],
instanceNodes: t.Statement[],
instanceNodes: t.ExpressionStatement[],
lastInstanceNodeReturnsThis: boolean,
pureStaticNodes: t.FunctionDeclaration[],
classBindingNode: t.Statement | null,
wrapClass: (path: NodePath<t.Class>) => NodePath;
Expand All @@ -258,6 +259,7 @@ export function createClassFeaturePlugin({
staticNodes,
pureStaticNodes,
instanceNodes,
lastInstanceNodeReturnsThis,
classBindingNode,
wrapClass,
} = buildFieldsInitNodes(
Expand All @@ -278,6 +280,7 @@ export function createClassFeaturePlugin({
staticNodes,
pureStaticNodes,
instanceNodes,
lastInstanceNodeReturnsThis,
classBindingNode,
wrapClass,
} = buildFieldsInitNodes(
Expand Down Expand Up @@ -308,6 +311,7 @@ export function createClassFeaturePlugin({
prop.traverse(referenceVisitor, state);
}
},
lastInstanceNodeReturnsThis,
);
}

Expand Down
16 changes: 13 additions & 3 deletions packages/babel-helper-create-class-features-plugin/src/misc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,9 @@ interface RenamerState {
export function injectInitialization(
path: NodePath<t.Class>,
constructor: NodePath<t.ClassMethod> | undefined,
nodes: t.Statement[],
nodes: t.ExpressionStatement[],
renamer?: (visitor: Visitor<RenamerState>, state: RenamerState) => void,
lastReturnsThis?: boolean,
) {
if (!nodes.length) return;

Expand Down Expand Up @@ -99,10 +100,19 @@ export function injectInitialization(
let isFirst = true;
for (const bareSuper of bareSupers) {
if (isFirst) {
bareSuper.insertAfter(nodes);
isFirst = false;
} else {
bareSuper.insertAfter(nodes.map(n => t.cloneNode(n)));
nodes = nodes.map(n => t.cloneNode(n));
}
if (!bareSuper.parentPath.isExpressionStatement()) {
const allNodes: t.Expression[] = [
bareSuper.node,
...nodes.map(n => t.toExpression(n)),
];
if (!lastReturnsThis) allNodes.push(t.thisExpression());
bareSuper.replaceWith(t.sequenceExpression(allNodes));
} else {
bareSuper.insertAfter(nodes);
}
}
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
class Foo extends Bar {
constructor(x = test ? (() => (super(), babelHelpers.defineProperty(this, "bar", "foo"), this))() : 0) {}
constructor(x = test ? (super(), babelHelpers.defineProperty(this, "bar", "foo")) : 0) {}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class Foo extends Bar {
constructor(x = () => {
check((super(), babelHelpers.defineProperty(this, "bar", "foo"), this));
check((super(), babelHelpers.defineProperty(this, "bar", "foo")));
}) {}
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
class Foo extends Bar {
constructor(x = (() => (super(), babelHelpers.defineProperty(this, "bar", "foo"), this))()) {}
constructor(x = (super(), babelHelpers.defineProperty(this, "bar", "foo"))) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ var Foo = /*#__PURE__*/function (_Bar) {
function Foo() {
var _this;
babelHelpers.classCallCheck(this, Foo);
foo((_this = _super.call(this), babelHelpers.defineProperty(babelHelpers.assertThisInitialized(_this), "bar", "foo"), babelHelpers.assertThisInitialized(_this)));
foo((_this = _super.call(this), babelHelpers.defineProperty(babelHelpers.assertThisInitialized(_this), "bar", "foo")));
return _this;
}
return babelHelpers.createClass(Foo);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class ComputedMethod extends Obj {
super();
expect(this.field).toBeUndefined();
}
[(super(), babelHelpers.defineProperty(this, "field", 1), this)]() {}
[(super(), babelHelpers.defineProperty(this, "field", 1))]() {}
}
expect(this.field).toBe(1);
new B();
Expand All @@ -57,7 +57,7 @@ new ComputedMethod();
class ComputedField extends Obj {
constructor() {
let _ref;
_ref = (super(), babelHelpers.defineProperty(this, "field", 1), this);
_ref = (super(), babelHelpers.defineProperty(this, "field", 1));
class B extends Obj {
constructor() {
super();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class A extends B {
x = 2;
constructor() {
x ? super(a) : super(b)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"plugins": ["transform-class-properties"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class A extends B {
constructor() {
x ? (super(a), babelHelpers.defineProperty(this, "x", 2)) : (super(b), babelHelpers.defineProperty(this, "x", 2));
}
}
9 changes: 6 additions & 3 deletions packages/babel-plugin-transform-typescript/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ export default declare((api, opts: Options) => {
// property is only added once. This is necessary for cases like
// using `transform-classes`, which causes this visitor to run
// twice.
const assigns = [];
const assigns: t.ExpressionStatement[] = [];
const { scope } = path;
for (const paramPath of path.get("params")) {
const param = paramPath.node;
Expand All @@ -226,8 +226,11 @@ export default declare((api, opts: Options) => {
"Parameter properties can not be destructuring patterns.",
);
}
assigns.push(template.statement.ast`
this.${t.cloneNode(id)} = ${t.cloneNode(id)}`);
assigns.push(
template.statement.ast`
this.${t.cloneNode(id)} = ${t.cloneNode(id)}
` as t.ExpressionStatement,
);

paramPath.replaceWith(paramPath.get("parameter"));
scope.registerBinding("param", paramPath);
Expand Down