Skip to content

Commit

Permalink
Decorators misc fixes (#14339)
Browse files Browse the repository at this point in the history
* fix: ensure initializer is invoked without parameters

* fix: assert class decorators return a callable or undefined

* fix: accessor decorator returns { initialize?: (initialValue: unknown) => unknown };

* fix: assert accessor decorator returns callable get/set/initialize

* add getter/setter test cases

* add accessor-initializers ordering test

* address review comments

* remove unused param base

* add leaked context addInitializer test
  • Loading branch information
JLHwung committed Mar 14, 2022
1 parent e597854 commit b636306
Show file tree
Hide file tree
Showing 33 changed files with 243 additions and 61 deletions.
2 changes: 1 addition & 1 deletion packages/babel-helpers/src/helpers-generated.ts

Large diffs are not rendered by default.

47 changes: 31 additions & 16 deletions packages/babel-helpers/src/helpers/applyDecs.js
Expand Up @@ -14,6 +14,8 @@
SETTER = 4;
STATIC = 5;
CLASS = 10; // only used in assertValidReturnValue
*/

function createMetadataMethodsForProperty(metadataMap, kind, property) {
Expand Down Expand Up @@ -120,13 +122,12 @@ function convertMetadataMapToFinal(obj, metadataMap) {

function createAddInitializerMethod(initializers) {
return function addInitializer(initializer) {
assertValidInitializer(initializer);
assertCallable(initializer, "An initializer");
initializers.push(initializer);
};
}

function memberDecCtx(
base,
name,
desc,
metadataMap,
Expand Down Expand Up @@ -207,9 +208,9 @@ function memberDecCtx(
);
}

function assertValidInitializer(initializer) {
if (typeof initializer !== "function") {
throw new Error("initializers must be functions");
function assertCallable(fn, hint) {
if (typeof fn !== "function") {
throw new TypeError(hint + " must be a function");
}
}

Expand All @@ -218,18 +219,29 @@ function assertValidReturnValue(kind, value) {

if (kind === 1 /* ACCESSOR */) {
if (type !== "object" || value === null) {
throw new Error(
throw new TypeError(
"accessor decorators must return an object with get, set, or initializer properties or void 0"
);
}
if (value.get !== undefined) {
assertCallable(value.get, "accessor.get");
}
if (value.set !== undefined) {
assertCallable(value.set, "accessor.set");
}
if (value.initialize !== undefined) {
assertCallable(value.initialize, "accessor.initialize");
}
} else if (type !== "function") {
var hint;
if (kind === 0 /* FIELD */) {
throw new Error(
"field decorators must return a initializer function or void 0"
);
hint = "field";
} else if (kind === 10 /* CLASS */) {
hint = "class";
} else {
throw new Error("method decorators must return a function or void 0");
hint = "method";
}
throw new TypeError(hint + " decorators must return a function or void 0");
}
}

Expand Down Expand Up @@ -285,7 +297,6 @@ function applyMemberDec(
}

var ctx = memberDecCtx(
base,
name,
desc,
metadataMap,
Expand All @@ -306,7 +317,7 @@ function applyMemberDec(
if (kind === 0 /* FIELD */) {
initializer = newValue;
} else if (kind === 1 /* ACCESSOR */) {
initializer = newValue.initializer;
initializer = newValue.initialize;

get = newValue.get || value.get;
set = newValue.set || value.set;
Expand All @@ -329,7 +340,7 @@ function applyMemberDec(
if (kind === 0 /* FIELD */) {
newInit = newValue;
} else if (kind === 1 /* ACCESSOR */) {
newInit = newValue.initializer;
newInit = newValue.initialize;

get = newValue.get || value.get;
set = newValue.set || value.set;
Expand Down Expand Up @@ -510,7 +521,7 @@ function pushInitializers(ret, initializers) {

ret.push(function (instance) {
for (var i = 0; i < initializers.length; i++) {
initializers[i].call(instance, instance);
initializers[i].call(instance);
}
return instance;
});
Expand Down Expand Up @@ -538,15 +549,19 @@ function applyClassDecs(ret, targetClass, metadataMap, classDecs) {
);

for (var i = classDecs.length - 1; i >= 0; i--) {
newClass = classDecs[i](newClass, ctx) || newClass;
var nextNewClass = classDecs[i](newClass, ctx);
if (nextNewClass !== undefined) {
assertValidReturnValue(10 /* CLASS */, nextNewClass);
newClass = nextNewClass;
}
}

ret.push(newClass);

if (initializers.length > 0) {
ret.push(function () {
for (var i = 0; i < initializers.length; i++) {
initializers[i].call(newClass, newClass);
initializers[i].call(newClass);
}
});
} else {
Expand Down
@@ -1,6 +1,6 @@
function dec({ get, set }, context) {
context.addInitializer((instance) => {
instance[context.name + 'Context'] = context;
context.addInitializer(function() {
this[context.name + 'Context'] = context;
});

return {
Expand All @@ -12,7 +12,7 @@ function dec({ get, set }, context) {
set.call(this, v + 1);
},

initializer(v) {
initialize(v) {
return v ? v : 1;
}
}
Expand Down
@@ -1,6 +1,6 @@
function dec({ get, set }, context) {
context.addInitializer((instance) => {
instance[context.name + 'Context'] = context;
context.addInitializer(function() {
this[context.name + 'Context'] = context;
});

return {
Expand All @@ -12,7 +12,7 @@ function dec({ get, set }, context) {
set.call(this, v + 1);
},

initializer(v) {
initialize(v) {
return v ? v : 1;
}
}
Expand Down
@@ -1,6 +1,6 @@
function dec({ get, set }, context) {
context.addInitializer((instance) => {
instance[context.name + 'Context'] = context;
context.addInitializer(function() {
this[context.name + 'Context'] = context;
});

return {
Expand All @@ -12,7 +12,7 @@ function dec({ get, set }, context) {
set.call(this, v + 1);
},

initializer(v) {
initialize(v) {
return v ? v : 1;
}
}
Expand Down
@@ -1,6 +1,6 @@
function dec({ get, set }, context) {
context.addInitializer((instance) => {
instance[context.name + 'Context'] = context;
context.addInitializer(function() {
this[context.name + 'Context'] = context;
});

return {
Expand All @@ -12,7 +12,7 @@ function dec({ get, set }, context) {
set.call(this, v + 1);
},

initializer(v) {
initialize(v) {
return v ? v : 1;
}
}
Expand Down
@@ -1,6 +1,6 @@
function dec(get, context) {
context.addInitializer((instance) => {
instance[context.name + 'Context'] = context;
context.addInitializer(function() {
this[context.name + 'Context'] = context;
});

return function () {
Expand Down
@@ -1,6 +1,6 @@
function dec(get, context) {
context.addInitializer((instance) => {
instance[context.name + 'Context'] = context;
context.addInitializer(function() {
this[context.name + 'Context'] = context;
});

return function () {
Expand Down
@@ -1,6 +1,6 @@
function dec(get, context) {
context.addInitializer((instance) => {
instance[context.name + 'Context'] = context;
context.addInitializer(function() {
this[context.name + 'Context'] = context;
});

return function () {
Expand Down
@@ -1,6 +1,6 @@
function dec(get, context) {
context.addInitializer((instance) => {
instance[context.name + 'Context'] = context;
context.addInitializer(function() {
this[context.name + 'Context'] = context;
});

return function () {
Expand Down
@@ -1,6 +1,6 @@
function dec(value, context) {
context.addInitializer((instance) => {
instance[context.name + '_' + context.kind + 'Context'] = context;
context.addInitializer(function() {
this[context.name + '_' + context.kind + 'Context'] = context;
});

if (context.kind === 'getter') {
Expand Down
@@ -1,6 +1,6 @@
function dec(value, context) {
context.addInitializer((instance) => {
instance[context.name + '_' + context.kind + 'Context'] = context;
context.addInitializer(function() {
this[context.name + '_' + context.kind + 'Context'] = context;
});

if (context.kind === 'getter') {
Expand Down
@@ -1,6 +1,6 @@
function dec(value, context) {
context.addInitializer((instance) => {
instance[context.name + '_' + context.kind + 'Context'] = context;
context.addInitializer(function() {
this[context.name + '_' + context.kind + 'Context'] = context;
});

if (context.kind === 'getter') {
Expand Down
@@ -1,6 +1,6 @@
function dec(value, context) {
context.addInitializer((instance) => {
instance[context.name + '_' + context.kind + 'Context'] = context;
context.addInitializer(function() {
this[context.name + '_' + context.kind + 'Context'] = context;
});

if (context.kind === 'getter') {
Expand Down
@@ -1,6 +1,6 @@
function dec(fn, context) {
context.addInitializer((instance) => {
instance[context.name + 'Context'] = context;
context.addInitializer(function() {
this[context.name + 'Context'] = context;
});

return function () {
Expand Down
@@ -1,6 +1,6 @@
function dec(fn, context) {
context.addInitializer((instance) => {
instance[context.name + 'Context'] = context;
context.addInitializer(function() {
this[context.name + 'Context'] = context;
});

return function () {
Expand Down
@@ -1,6 +1,6 @@
function dec(fn, context) {
context.addInitializer((instance) => {
instance[context.name + 'Context'] = context;
context.addInitializer(function() {
this[context.name + 'Context'] = context;
});

return function () {
Expand Down
@@ -1,6 +1,6 @@
function dec(fn, context) {
context.addInitializer((instance) => {
instance[context.name + 'Context'] = context;
context.addInitializer(function() {
this[context.name + 'Context'] = context;
});

return function () {
Expand Down
@@ -0,0 +1,21 @@
let addInitializer, i = 0;
const logs = [];

function decCallProtoAddInitializer() {
addInitializer(() => logs.push(i++));
}

function decMethod(_, context) {
({ addInitializer } = context);
addInitializer(() => logs.push(i++));
}

@decCallProtoAddInitializer
class C {
@decMethod m() {}
@decCallProtoAddInitializer static n() {}
}

new C;

expect(logs).toEqual([0, 1]);
@@ -0,0 +1,21 @@
let addInitializer, i = 0;
const logs = [];

function decCallStaticAddInitializer() {
addInitializer(() => logs.push(i++));
}

function decStaticMethod(_, context) {
({ addInitializer } = context);
addInitializer(() => logs.push(i++));
}

@decCallStaticAddInitializer
class C {
@decStaticMethod static m() {}
@decCallStaticAddInitializer n() {}
}

new C;

expect(logs).toEqual([0, 1]);
@@ -0,0 +1,46 @@
var log = [];

function push(x) { log.push(x); return x; }

function logClassDecoratorRun(a, b, c) {
push(a);
return function (el, { addInitializer }) {
push(b);
addInitializer(function () { push(c); });
return el;
};
}

function logAccessorDecoratorRun(a, b, c, d) {
push(a);
return function (el, { addInitializer }) {
push(b);
addInitializer(function () { push(c); });
return {
initialize: () => push(d)
};
};
}

@logClassDecoratorRun(0, 19, 29)
@logClassDecoratorRun(1, 18, 28)
class A {
@logAccessorDecoratorRun(2, 11)
@logAccessorDecoratorRun(3, 10)
accessor a;

@logAccessorDecoratorRun(4, 13, 21, 25)
@logAccessorDecoratorRun(5, 12, 20, 24)
static accessor b;

@logAccessorDecoratorRun(6, 15, 23, 27)
@logAccessorDecoratorRun(7, 14, 22, 26)
static accessor #c;

@logAccessorDecoratorRun(8, 17)
@logAccessorDecoratorRun(9, 16)
accessor #d;
}

var nums = Array.from({ length: 30 }, (_, i) => i);
expect(log).toEqual(nums);

0 comments on commit b636306

Please sign in to comment.