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

Decorators misc fixes #14339

Merged
merged 9 commits into from Mar 14, 2022
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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are renaming a property here, I wonder if it makes sense to also support the old one with a warning:

if ((initializer = newValue.init) == null && (initializer = newValue.initializer) && typeof console !== "undefined") {
  console.warn(".initializer has been renamed to .init as of March 2022")
}

@pzuraq This idea mostly came up while I was reviewing #14353, since it's not just a "we implemented the spec wrongly" anymore but a "the proposal had a change that made all the accessor initializers usages invalid".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that seems reasonable to me, will update

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, feel free to review/approve this PR since you are the "expert in the field" for decorators! However, I'd like to wait merging this until #14353 is ready so that they can go in the same release.


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);