Skip to content

Commit

Permalink
Fix decorated class computed keys ordering (#16350)
Browse files Browse the repository at this point in the history
* fix: memoise computed keys in decorated class

* Node 6 does not support async functions

* add input tests
  • Loading branch information
JLHwung committed Mar 15, 2024
1 parent fc0d5ad commit 1512960
Show file tree
Hide file tree
Showing 11 changed files with 969 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1219,6 +1219,7 @@ function transformClass(
let classDecorationsFlag = 0;
let classDecorations: t.Expression[] = [];
let classDecorationsId: t.Identifier;
let computedKeyAssignments: t.AssignmentExpression[] = [];
if (classDecorators) {
classInitLocal = generateLetUidIdentifier(scopeParent, "initClass");
needsDeclaraionForClassBinding = path.isClassDeclaration();
Expand Down Expand Up @@ -1249,6 +1250,40 @@ function transformClass(
classAssignments,
);
}

if (!hasElementDecorators) {
// Sync body paths as non-decorated computed accessors have been transpiled
// to getter-setter pairs.
for (const element of path.get("body.body")) {
const { node } = element;
const isComputed = "computed" in node && node.computed;
if (isComputed) {
if (element.isClassProperty({ static: true })) {
if (!element.get("key").isConstantExpression()) {
const key = (node as t.ClassProperty).key;
const maybeAssignment = memoiseComputedKey(
key,
scopeParent,
scopeParent.generateUid("computedKey"),
);
if (maybeAssignment != null) {
// If it is a static computed field within a decorated class, we move the computed key
// into `computedKeyAssignments` which will be then moved into the non-static class,
// to ensure that the evaluation order and private environment are correct
node.key = t.cloneNode(maybeAssignment.left);
computedKeyAssignments.push(maybeAssignment);
}
}
} else if (computedKeyAssignments.length > 0) {
prependExpressionsToComputedKey(
computedKeyAssignments,
element as NodePath<ClassElementCanHaveComputedKeys>,
);
computedKeyAssignments = [];
}
}
}
}
} else {
if (!path.node.id) {
path.node.id = path.scope.generateUidIdentifier("Class");
Expand All @@ -1259,7 +1294,6 @@ function transformClass(
let lastInstancePrivateName: t.PrivateName;
let needsInstancePrivateBrandCheck = false;

let computedKeyAssignments: t.AssignmentExpression[] = [];
let fieldInitializerExpressions = [];
let staticFieldInitializerExpressions: t.Expression[] = [];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,72 @@
}
expect([...A].map(fn => fn(b)).join()).toBe("B#x,B#x");
}

{
"different private/super access in a non-decorated static method";

let b;

const idFactory = (hint) => {
return class { static ["id" + hint](v) { return v } }
}

class C extends idFactory("C") { #x = "C#x"; }

const dec = (b) => {
originalB = b;
return C;
}

class A extends idFactory("A") {
#x = "A#x";
static *[Symbol.iterator]() {
@dec
class B extends idFactory("B") {
#x = "B#x";
static [(yield super.idA(o => o.#x), Symbol.iterator)] = function* () {
yield (o => o.#x);
}
}
b = new originalB();
yield* B;
}
}
expect([...A].map(fn => fn(b)).join()).toBe("B#x,B#x");
}

{
"different private/super access in a decorated static method";

const dummy = () => {};

let b;

const idFactory = (hint) => {
return class { static ["id" + hint](v) { return v } }
}

class C extends idFactory("C") { #x = "C#x"; }

const dec = (b) => {
originalB = b;
return C;
}

class A extends idFactory("A") {
#x = "A#x";
static *[Symbol.iterator]() {
@dec
class B extends idFactory("B") {
#x = "B#x";
@(yield super.idA(o => o.#x), dummy)
static [(yield super.idA(o => o.#x), Symbol.iterator)] = function* () {
yield (o => o.#x);
}
}
b = new originalB();
yield* B;
}
}
expect([...A].map(fn => fn(b)).join()).toBe("B#x,B#x,B#x");
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,72 @@
}
expect([...A].map(fn => fn(b)).join()).toBe("B#x,B#x");
}

{
"different private/super access in a non-decorated static method";

let b;

const idFactory = (hint) => {
return class { static ["id" + hint](v) { return v } }
}

class C extends idFactory("C") { #x = "C#x"; }

const dec = (b) => {
originalB = b;
return C;
}

class A extends idFactory("A") {
#x = "A#x";
static *[Symbol.iterator]() {
@dec
class B extends idFactory("B") {
#x = "B#x";
static [(yield super.idA(o => o.#x), Symbol.iterator)] = function* () {
yield (o => o.#x);
}
}
b = new originalB();
yield* B;
}
}
expect([...A].map(fn => fn(b)).join()).toBe("B#x,B#x");
}

{
"different private/super access in a decorated static method";

const dummy = () => {};

let b;

const idFactory = (hint) => {
return class { static ["id" + hint](v) { return v } }
}

class C extends idFactory("C") { #x = "C#x"; }

const dec = (b) => {
originalB = b;
return C;
}

class A extends idFactory("A") {
#x = "A#x";
static *[Symbol.iterator]() {
@dec
class B extends idFactory("B") {
#x = "B#x";
@(yield super.idA(o => o.#x), dummy)
static [(yield super.idA(o => o.#x), Symbol.iterator)] = function* () {
yield (o => o.#x);
}
}
b = new originalB();
yield* B;
}
}
expect([...A].map(fn => fn(b)).join()).toBe("B#x,B#x,B#x");
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
const classDec1 = (log) => (cls, ctxClass) => {
log.push("c2");
ctxClass.addInitializer(() => log.push("c5"));
ctxClass.addInitializer(() => log.push("c6"));
};
const classDec2 = (log) => (cls, ctxClass) => {
log.push("c1");
ctxClass.addInitializer(() => log.push("c3"));
ctxClass.addInitializer(() => log.push("c4"));
};

{
const log = [];

@classDec1(log)
@classDec2(log)
class C {
[log.push("k1")];
[log.push("k2")];
[log.push("k3")];
}

expect(log.join()).toBe(
"k1,k2,k3," + // ClassElementEvaluation
"c1,c2," + // ApplyDecoratorsToClassDefinition
"c3,c4,c5,c6", // classExtraInitializers
);
}

{
const log = [];

@classDec1(log)
@classDec2(log)
class C {
static [log.push("k1")];
async [log.push("k2")](v) {};
[log.push("k3")];
}

expect(log.join()).toBe(
"k1,k2,k3," + // ClassElementEvaluation
"c1,c2," + // ApplyDecoratorsToClassDefinition
"c3,c4,c5,c6", // classExtraInitializers
);
}

{
const log = [];

@classDec1(log)
@classDec2(log)
class C {
get [log.push("k1")]() {};
static [log.push("k2")];
[log.push("k3")];
}

expect(log.join()).toBe(
"k1,k2,k3," + // ClassElementEvaluation
"c1,c2," + // ApplyDecoratorsToClassDefinition
"c3,c4,c5,c6", // classExtraInitializers
);
}

{
const log = [];

@classDec1(log)
@classDec2(log)
class C {
[log.push("k1")];
accessor [log.push("k2")];
static [log.push("k3")];
}

expect(log.join()).toBe(
"k1,k2,k3," + // ClassElementEvaluation
"c1,c2," + // ApplyDecoratorsToClassDefinition
"c3,c4,c5,c6", // classExtraInitializers
);
}

{
const log = [];

@classDec1(log)
@classDec2(log)
class C {
static set [log.push("k1")](v) {};
[log.push("k2")];
static [log.push("k3")];
}

expect(log.join()).toBe(
"k1,k2,k3," + // ClassElementEvaluation
"c1,c2," + // ApplyDecoratorsToClassDefinition
"c3,c4,c5,c6", // classExtraInitializers
);
}

{
const log = [];

@classDec1(log)
@classDec2(log)
class C {
static [log.push("k1")];
static [log.push("k2")];
static [log.push("k3")];
}

expect(log.join()).toBe(
"k1,k2,k3," + // ClassElementEvaluation
"c1,c2," + // ApplyDecoratorsToClassDefinition
"c3,c4,c5,c6", // classExtraInitializers
);
}

0 comments on commit 1512960

Please sign in to comment.