Skip to content

Commit

Permalink
Fix decorator bare yield await (#16484)
Browse files Browse the repository at this point in the history
* refactor: use usesFunctionContextOrYieldAwait on decorator

* fix: don't inline class decorators if containing fn contexts or yield/await

* add test cases
  • Loading branch information
JLHwung committed May 9, 2024
1 parent db3e4f5 commit c1bf7d7
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -664,13 +664,13 @@ type GenerateDecorationListResult = {
/**
* Zip decorators and decorator this values into an array
*
* @param {t.Expression[]} decorators
* @param {t.Decorator[]} decorators
* @param {((t.Expression | undefined)[])} decoratorsThis decorator this values
* @param {DecoratorVersionKind} version
* @returns {GenerateDecorationListResult}
*/
function generateDecorationList(
decorators: t.Expression[],
decorators: t.Decorator[],
decoratorsThis: (t.Expression | undefined)[],
version: DecoratorVersionKind,
): GenerateDecorationListResult {
Expand All @@ -687,7 +687,7 @@ function generateDecorationList(
decoratorsThis[i] || t.unaryExpression("void", t.numericLiteral(0)),
);
}
decs.push(decorators[i]);
decs.push(decorators[i].expression);
}

return { haveThis: haveOneThis, decs };
Expand Down Expand Up @@ -1031,12 +1031,12 @@ function transformClass(
let protoInitLocal: t.Identifier;
let staticInitLocal: t.Identifier;
const classIdName = path.node.id?.name;
// Check if the expression does not reference function-specific
// context or the given identifier name.
// Check if the decorator does not reference function-specific
// context or the given identifier name or contains yield or await expression.
// `true` means "maybe" and `false` means "no".
const usesFunctionContextOrYieldAwait = (expression: t.Node) => {
const usesFunctionContextOrYieldAwait = (decorator: t.Decorator) => {
try {
t.traverseFast(expression, node => {
t.traverseFast(decorator, node => {
if (
t.isThisExpression(node) ||
t.isSuper(node) ||
Expand Down Expand Up @@ -1172,20 +1172,19 @@ function transformClass(
let decoratorReceiverId: t.Identifier | null = null;

// Memoise the this value `a.b` of decorator member expressions `@a.b.dec`,
type HandleDecoratorExpressionsResult = {
type HandleDecoratorsResult = {
// whether the whole decorator list requires memoisation
hasSideEffects: boolean;
usesFnContext: boolean;
// the this value of each decorator if applicable
decoratorsThis: (t.Expression | undefined)[];
};
function handleDecoratorExpressions(
expressions: t.Expression[],
): HandleDecoratorExpressionsResult {
function handleDecorators(decorators: t.Decorator[]): HandleDecoratorsResult {
let hasSideEffects = false;
let usesFnContext = false;
const decoratorsThis: (t.Expression | null)[] = [];
for (const expression of expressions) {
for (const decorator of decorators) {
const { expression } = decorator;
let object;
if (
(version === "2023-11" ||
Expand All @@ -1208,7 +1207,7 @@ function transformClass(
}
decoratorsThis.push(object);
hasSideEffects ||= !scopeParent.isStatic(expression);
usesFnContext ||= usesFunctionContextOrYieldAwait(expression);
usesFnContext ||= usesFunctionContextOrYieldAwait(decorator);
}
return { hasSideEffects, usesFnContext, decoratorsThis };
}
Expand All @@ -1231,20 +1230,20 @@ function transformClass(

path.node.decorators = null;

const decoratorExpressions = classDecorators.map(el => el.expression);
const classDecsUsePrivateName = decoratorExpressions.some(usesPrivateField);
const { hasSideEffects, decoratorsThis } =
handleDecoratorExpressions(decoratorExpressions);
const classDecsUsePrivateName = classDecorators.some(usesPrivateField);
const { hasSideEffects, usesFnContext, decoratorsThis } =
handleDecorators(classDecorators);

const { haveThis, decs } = generateDecorationList(
decoratorExpressions,
classDecorators,
decoratorsThis,
version,
);
classDecorationsFlag = haveThis ? 1 : 0;
classDecorations = decs;

if (
usesFnContext ||
(hasSideEffects && willExtractSomeElemDecs) ||
classDecsUsePrivateName
) {
Expand Down Expand Up @@ -1341,11 +1340,10 @@ function transformClass(
let decoratorsHaveThis;

if (hasDecorators) {
const decoratorExpressions = decorators.map(d => d.expression);
const { hasSideEffects, usesFnContext, decoratorsThis } =
handleDecoratorExpressions(decoratorExpressions);
handleDecorators(decorators);
const { decs, haveThis } = generateDecorationList(
decoratorExpressions,
decorators,
decoratorsThis,
version,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,24 @@
expect(receivedNewTarget).toBe(B);
}

{
let receivedNewTarget;
function decFactory(target) {
receivedNewTarget = target;
return x => x;
}
function B() {
@decFactory(new.target)
class C {
#p;
}
}

new B();

expect(receivedNewTarget).toBe(B);
}

{
function noop() {}
let receivedNewTarget;
Expand Down Expand Up @@ -52,3 +70,17 @@

expect(receivedNewTarget).toBe(B);
}

{
let C;
const newC = class {};
function B () {
C = @(new.target)
class {
#p;
}
}

Reflect.construct(B, [], function () { return newC })
expect(C).toBe(newC);
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,25 @@
expect(receivedName).toBe("B");
}

{
let receivedName;
function decFactory(name) {
receivedName = name;
return x => x;
}
class B {
static m() {
@decFactory(this.name)
class C {
#p;
}
}
}

B.m();
expect(receivedName).toBe("B");
}

{
let receivedLength;
function decFactory(length) {
Expand Down Expand Up @@ -59,3 +78,18 @@
B.m();
expect(receivedLength).toBe(2);
}

{
let C;
const newC = class {};
const B = () => newC;
B.m = function () {
C = @(this)
class {
#p;
}
}

B.m();
expect(C).toBe(newC);
}
Original file line number Diff line number Diff line change
Expand Up @@ -100,3 +100,63 @@ const noop = () => {};
expect([...log].join()).toBe("0,1,2,3,4,5,6,7,8,9,10,11");
expect(Foo).toHaveProperty("accessor");
}

{
const id = function* (x) {
yield x;
}
const log = {
*[Symbol.iterator]() {
@(yield 0, noop)
@(yield 1, noop)
class Foo {
method() {}
static method() {}
field;
static field;
get getter() {
return;
}
static get getter() {
return;
}
set setter(x) {}
static set setter(x) {}
accessor accessor;
static accessor accessor;
}
},
};
expect([...log].join()).toBe("0,1");
}

{
let C;
const iter = (function* iterFactory() {
C =
@(yield)
@(yield)
class C {
method() {}
static method() {}
field;
static field;
get getter() {
return;
}
static get getter() {
return;
}
set setter(x) {}
static set setter(x) {}
accessor accessor;
static accessor accessor;
};
})();
const C1 = class {},
C2 = class {};
iter.next();
iter.next(() => C1);
iter.next(() => C2);
expect(C).toBe(C1);
}

0 comments on commit c1bf7d7

Please sign in to comment.