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

Fix decorator bare yield await #16484

Merged
merged 3 commits into from
May 9, 2024
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 @@ -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);
}
Loading