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

Do not define access.get for public setter decorators #16265

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/babel-helpers/src/helpers-generated.ts
Expand Up @@ -48,10 +48,10 @@ export default Object.freeze({
"7.21.0",
'import checkInRHS from"checkInRHS";import setFunctionName from"setFunctionName";import toPropertyKey from"toPropertyKey";export default function applyDecs2305(e,t,r,n,o,a){function i(e,t,r){return function(n,o){return r&&r(n),e[t].call(n,o)}}function c(e,t){for(var r=0;r<e.length;r++)e[r].call(t);return t}function s(e,t,r,n){if("function"!=typeof e&&(n||void 0!==e))throw new TypeError(t+" must "+(r||"be")+" a function"+(n?"":" or undefined"));return e}function applyDec(e,t,r,n,o,a,c,u,l,f,p,d,h){function m(e){if(!h(e))throw new TypeError("Attempted to access private element on non-instance")}var y,v=t[0],g=t[3],b=!u;if(!b){r||Array.isArray(v)||(v=[v]);var w={},S=[],A=3===o?"get":4===o||d?"set":"value";f?(p||d?w={get:setFunctionName((function(){return g(this)}),n,"get"),set:function(e){t[4](this,e)}}:w[A]=g,p||setFunctionName(w[A],n,2===o?"":A)):p||(w=Object.getOwnPropertyDescriptor(e,n))}for(var P=e,j=v.length-1;j>=0;j-=r?2:1){var D=v[j],E=r?v[j-1]:void 0,I={},O={kind:["field","accessor","method","getter","setter","class"][o],name:n,metadata:a,addInitializer:function(e,t){if(e.v)throw new Error("attempted to call addInitializer after decoration was finished");s(t,"An initializer","be",!0),c.push(t)}.bind(null,I)};try{if(b)(y=s(D.call(E,P,O),"class decorators","return"))&&(P=y);else{var k,F;O.static=l,O.private=f,f?2===o?k=function(e){return m(e),w.value}:(o<4&&(k=i(w,"get",m)),3!==o&&(F=i(w,"set",m))):(k=function(e){return e[n]},(o<2||4===o)&&(F=function(e,t){e[n]=t}));var N=O.access={has:f?h.bind():function(e){return n in e}};if(k&&(N.get=k),F&&(N.set=F),P=D.call(E,d?{get:w.get,set:w.set}:w[A],O),d){if("object"==typeof P&&P)(y=s(P.get,"accessor.get"))&&(w.get=y),(y=s(P.set,"accessor.set"))&&(w.set=y),(y=s(P.init,"accessor.init"))&&S.push(y);else if(void 0!==P)throw new TypeError("accessor decorators must return an object with get, set, or init properties or void 0")}else s(P,(p?"field":"method")+" decorators","return")&&(p?S.push(P):w[A]=P)}}finally{I.v=!0}}return(p||d)&&u.push((function(e,t){for(var r=S.length-1;r>=0;r--)t=S[r].call(e,t);return t})),p||b||(f?d?u.push(i(w,"get"),i(w,"set")):u.push(2===o?w[A]:i.call.bind(w[A])):Object.defineProperty(e,n,w)),P}function u(e,t){return Object.defineProperty(e,Symbol.metadata||Symbol.for("Symbol.metadata"),{configurable:!0,enumerable:!0,value:t})}if(arguments.length>=6)var l=a[Symbol.metadata||Symbol.for("Symbol.metadata")];var f=Object.create(null==l?null:l),p=function(e,t,r,n){var o,a,i=[],s=function(t){return checkInRHS(t)===e},u=new Map;function l(e){e&&i.push(c.bind(null,e))}for(var f=0;f<t.length;f++){var p=t[f];if(Array.isArray(p)){var d=p[1],h=p[2],m=p.length>3,y=16&d,v=!!(8&d),g=0==(d&=7),b=h+"/"+v;if(!g&&!m){var w=u.get(b);if(!0===w||3===w&&4!==d||4===w&&3!==d)throw new Error("Attempted to decorate a public method/accessor that has the same name as a previously decorated public method/accessor. This is not currently supported by the decorators plugin. Property name was: "+h);u.set(b,!(d>2)||d)}applyDec(v?e:e.prototype,p,y,m?"#"+h:toPropertyKey(h),d,n,v?a=a||[]:o=o||[],i,v,m,g,1===d,v&&m?s:r)}}return l(o),l(a),i}(e,t,o,f);return r.length||u(e,f),{e:p,get c(){var t=[];return r.length&&[u(applyDec(e,[r],n,e.name,5,f,t),f),c.bind(null,t,e)]}}}',
),
// size: 2882, gzip size: 1463
// size: 2883, gzip size: 1472
applyDecs2311: helper(
"7.23.0",
'import checkInRHS from"checkInRHS";import setFunctionName from"setFunctionName";import toPropertyKey from"toPropertyKey";export default function applyDecs2311(e,t,n,r,o,i){var c,a,u=Symbol.metadata||Symbol.for("Symbol.metadata"),s=Object.defineProperty,l={},f=n.length;function p(e,t,n,r){for(var o=0;o<e.length;o++)r=e[o].apply(n,t?[r]:[]);return t?r:n}function d(e,t,n,r){if("function"!=typeof e&&(r||void 0!==e))throw new TypeError(t+" must "+(n||"be")+" a function"+(r?"":" or undefined"));return e}function applyDec(e,t,n,r,o,i,u,f,m,h,y,v){function g(e){if(!v(e))throw new TypeError("Attempted to access private element on non-instance")}var b=[].concat(t[0]),w=t[3],D=!u,S=r+"/"+f,j=3===o,E=4===o,I=2===o;function P(e){if(!H&&!h&&!m){if(l[S+e])throw new Error("Decorating two elements with the same name ("+r+") is not supported yet");l[S+e]=1}}function k(e,t){return function(n,r){return t&&t(n),F[e].call(n,r)}}if(!D){var F={},N=[],O=j?"get":E||y?"set":"value";m?(h||y?F={get:setFunctionName((function(){return w(this)}),r,"get"),set:function(e){t[4](this,e)}}:F[O]=w,h||setFunctionName(F[O],r,I?"":O)):h||(F=Object.getOwnPropertyDescriptor(e,r))}for(var z=e,H=b.length-1;H>=0;H-=n?2:1){var K=b[H],R=n?b[H-1]:void 0,T={},A={kind:["field","accessor","method","getter","setter","class"][o],name:r,metadata:c,addInitializer:function(e,t){if(e.v)throw new Error("attempted to call addInitializer after decoration was finished");d(t,"An initializer","be",!0),i.push(t)}.bind(null,T)};if(D)a=K.call(R,z,A),T.v=1,d(a,"class decorators","return")&&(z=a);else if(A.static=f,A.private=m,a=A.access={has:m?v.bind():function(e){return r in e}},E||(P(),a.get=m?I?function(e){return g(e),F.value}:k("get",g):function(e){return e[r]}),j||(P(1),a.set=m?k("set",g):function(e,t){e[r]=t}),z=K.call(R,y?{get:F.get,set:F.set}:F[O],A),T.v=1,y){if("object"==typeof z&&z)(a=d(z.get,"accessor.get"))&&(F.get=a),(a=d(z.set,"accessor.set"))&&(F.set=a),(a=d(z.init,"accessor.init"))&&N.unshift(a);else if(void 0!==z)throw new TypeError("accessor decorators must return an object with get, set, or init properties or undefined")}else d(z,(h?"field":"method")+" decorators","return")&&(h?N.unshift(z):F[O]=z)}return o<2&&u.push(p.bind(null,N,1),p.bind(null,i,0)),h||D||(m?y?u.splice(-1,0,k("get"),k("set")):u.push(I?F[O]:p.call.bind(F[O])):s(e,r,F)),z}function m(e){return s(e,u,{configurable:!0,enumerable:!0,value:c})}return void 0!==i&&(c=i[u]),c=Object.create(null==c?null:c),a=function(){var n,r,i=[];function c(e){e&&i.push(p.bind(null,e,0))}for(var a=0;a<t.length;a++){var u=t[a],s=u[1],l=u[2],f=!!u[3],d=16&s,m=!!(8&s),h=0==(s&=7),y=1===s;applyDec(m?e:e.prototype,u,d,f?"#"+l:toPropertyKey(l),s,h||y?[]:m?r=r||[]:n=n||[],i,m,f,h,y,m&&f?function(t){return checkInRHS(t)===e}:o)}return c(n),c(r),i}(),f||m(e),{e:a,get c(){var t=[];return f&&[m(applyDec(e,[n],r,e.name,5,t)),p.bind(null,t,0,e)]}}}',
'import checkInRHS from"checkInRHS";import setFunctionName from"setFunctionName";import toPropertyKey from"toPropertyKey";export default function applyDecs2311(e,t,n,r,o,i){var a,c,u=Symbol.metadata||Symbol.for("Symbol.metadata"),s=Object.defineProperty,l=Object.create,f=l(null),p=n.length;function d(e,t,n,r){for(var o=0;o<e.length;o++)r=e[o].apply(n,t?[r]:[]);return t?r:n}function m(e,t,n,r){if("function"!=typeof e&&(r||void 0!==e))throw new TypeError(t+" must "+(n||"be")+" a function"+(r?"":" or undefined"));return e}function applyDec(e,t,n,r,o,i,u,l,p,h,y,v){function g(e){if(!v(e))throw new TypeError("Attempted to access private element on non-instance")}var b=[].concat(t[0]),w=t[3],D=!u,S=r+"/"+l,j=3===o,E=4===o,I=2===o;function P(e,t){return function(n,r){return t&&t(n),k[e].call(n,r)}}if(!D){var k={},F=[],N=j?"get":E||y?"set":"value";if(p?(h||y?k={get:setFunctionName((function(){return w(this)}),r,"get"),set:function(e){t[4](this,e)}}:k[N]=w,h||setFunctionName(k[N],r,I?"":N)):h||(k=Object.getOwnPropertyDescriptor(e,r)),!h&&!p){if((c=f[S])&&7!=(c^o))throw new Error("Decorating two elements with the same name ("+r+") is not supported yet");f[S]=o<3?1:o}}for(var O=e,z=b.length-1;z>=0;z-=n?2:1){var H=b[z],K=n?b[z-1]:void 0,R={},T={kind:["field","accessor","method","getter","setter","class"][o],name:r,metadata:a,addInitializer:function(e,t){if(e.v)throw new Error("attempted to call addInitializer after decoration was finished");m(t,"An initializer","be",!0),i.push(t)}.bind(null,R)};if(D)c=H.call(K,O,T),R.v=1,m(c,"class decorators","return")&&(O=c);else if(T.static=l,T.private=p,c=T.access={has:p?v.bind():function(e){return r in e}},E||(c.get=p?I?function(e){return g(e),k.value}:P("get",g):function(e){return e[r]}),I||j||(c.set=p?P("set",g):function(e,t){e[r]=t}),O=H.call(K,y?{get:k.get,set:k.set}:k[N],T),R.v=1,y){if("object"==typeof O&&O)(c=m(O.get,"accessor.get"))&&(k.get=c),(c=m(O.set,"accessor.set"))&&(k.set=c),(c=m(O.init,"accessor.init"))&&F.unshift(c);else if(void 0!==O)throw new TypeError("accessor decorators must return an object with get, set, or init properties or undefined")}else m(O,(h?"field":"method")+" decorators","return")&&(h?F.unshift(O):k[N]=O)}return o<2&&u.push(d.bind(null,F,1),d.bind(null,i,0)),h||D||(p?y?u.splice(-1,0,P("get"),P("set")):u.push(I?k[N]:d.call.bind(k[N])):s(e,r,k)),O}function h(e){return s(e,u,{configurable:!0,enumerable:!0,value:a})}return void 0!==i&&(a=i[u]),a=l(null==a?null:a),c=function(){var n,r,i=[];function a(e){e&&i.push(d.bind(null,e,0))}for(var c=0;c<t.length;c++){var u=t[c],s=u[1],l=u[2],f=!!u[3],p=16&s,m=!!(8&s),h=0==(s&=7),y=1===s;applyDec(m?e:e.prototype,u,p,f?"#"+l:toPropertyKey(l),s,h||y?[]:m?r=r||[]:n=n||[],i,m,f,h,y,m&&f?function(t){return checkInRHS(t)===e}:o)}return a(n),a(r),i}(),p||h(e),{e:c,get c(){var t=[];return p&&[h(applyDec(e,[n],r,e.name,5,t)),d.bind(null,t,0,e)]}}}',
),
// size: 544, gzip size: 300
asyncGeneratorDelegate: helper(
Expand Down
50 changes: 30 additions & 20 deletions packages/babel-helpers/src/helpers/applyDecs2311.ts
Expand Up @@ -199,8 +199,16 @@ export default /* @no-mangle */ function applyDecs2311(
) {
var symbolMetadata = Symbol.metadata || Symbol.for("Symbol.metadata");
var defineProperty = Object.defineProperty;
var create = Object.create;
var metadata: any;
var existingNonFields: Record<string, number> = {};
// Use both as and satisfies to ensure that we only use non-zero values
var existingNonFields = create(null) as Record<
Copy link
Member

Choose a reason for hiding this comment

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

I thought about this, since we use / splicing it should be safe.
But I don't have a strong preference for it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer this in case somebody injects stuff on the prototype.

string,
1 | 3 | 4
> satisfies Record<
string,
PROP_KIND.ACCESSOR | PROP_KIND.GETTER | PROP_KIND.SETTER
>;
var hasClassDecs = classDecs.length;
// This is a temporary variable for smaller helper size
var _: any;
Expand Down Expand Up @@ -269,21 +277,6 @@ export default /* @no-mangle */ function applyDecs2311(
var isSetter = kind === PROP_KIND.SETTER;
Copy link
Contributor

@JLHwung JLHwung Feb 7, 2024

Choose a reason for hiding this comment

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

Aside: Commenting on Line 273 mapKey = name + "/" + isStatic;:

It doesn't work when the computed key is a symbol. In the following example, Babel throws "Cannot convert a Symbol value to a string":

var dec = x => x;

class C {
  @dec
  *[Symbol.iterator]() {}
}

we can fix that in a new PR.

var isMethod = kind === PROP_KIND.METHOD;

function markExistingNonField(hasSetter?: 1) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This function is now only called once, so I inlined it where it's used.

// eslint-disable-next-line @typescript-eslint/no-use-before-define
if (!i && !isField && !isPrivate) {
if (existingNonFields[mapKey + hasSetter]) {
throw new Error(
"Decorating two elements with the same name (" +
name +
") is not supported yet",
);
}

existingNonFields[mapKey + hasSetter] = 1;
}
}

function _bindPropCall(name: keyof PropertyDescriptor, before?: Function) {
return function (_this: any, value?: any) {
if (before) {
Expand Down Expand Up @@ -327,6 +320,25 @@ export default /* @no-mangle */ function applyDecs2311(
} else if (!isField) {
desc = Object.getOwnPropertyDescriptor(Class, name);
}

if (!isField && !isPrivate) {
_ = existingNonFields[mapKey];
// flag is 1, 3, or 4; kind is 0, 1, 2, 3, or 4
// flag ^ kind is 7 if and only if one of them is 3 and the other one is 4.
if (_ && (_ ^ kind) !== 7) {
throw new Error(
"Decorating two elements with the same name (" +
name +
") is not supported yet",
);
}
// We use PROP_KIND.ACCESSOR to mark a name as "fully used":
// either a get/set pair, or a non-getter/setter.
existingNonFields[mapKey] =
kind < PROP_KIND.GETTER
? PROP_KIND.ACCESSOR
: (kind as PROP_KIND.GETTER | PROP_KIND.SETTER);
}
}

var newValue = Class;
Expand Down Expand Up @@ -378,7 +390,6 @@ export default /* @no-mangle */ function applyDecs2311(
};

if (!isSetter) {
markExistingNonField();
_.get = isPrivate
? isMethod
? function (_this: any) {
Expand All @@ -390,8 +401,7 @@ export default /* @no-mangle */ function applyDecs2311(
return target[name];
};
}
if (!isGetter) {
markExistingNonField(1);
if (!isMethod && !isGetter) {
_.set = isPrivate
? _bindPropCall("set", assertInstanceIfPrivate)
: function (target: any, v: any) {
Expand Down Expand Up @@ -540,7 +550,7 @@ export default /* @no-mangle */ function applyDecs2311(
if (parentClass !== undefined) {
metadata = parentClass[symbolMetadata];
}
metadata = Object.create(metadata == null ? null : metadata);
metadata = create(metadata == null ? null : metadata);
_ = applyMemberDecs();
if (!hasClassDecs) defineMetadata(targetClass);
return {
Expand Down
Expand Up @@ -26,6 +26,8 @@ let foo = new Foo();

const aContext = foo['#aContext'];

expect(aContext.access).not.toHaveProperty("set");

expect(aContext.access.has(foo)).toBe(true);
expect(aContext.access.has({})).toBe(false);
expect(aContext.access.has(Object.create(foo))).toBe(false);
Expand Down
Expand Up @@ -28,6 +28,9 @@ let foo = new Foo();
const aContext = foo['aContext'];
const bContext = foo['bContext'];

expect(aContext.access).not.toHaveProperty("set");
expect(bContext.access).not.toHaveProperty("set");

expect(aContext.access.has(foo)).toBe(true);
expect(aContext.access.has({})).toBe(false);
expect(aContext.access.has(Object.create(foo))).toBe(true);
Expand Down
Expand Up @@ -24,6 +24,8 @@ class Foo {

const aContext = Foo['#aContext'];

expect(aContext.access).not.toHaveProperty("set");

expect(aContext.access.has(Foo)).toBe(true);
expect(aContext.access.has({})).toBe(false);
expect(aContext.access.has(Object.create(Foo))).toBe(false);
Expand Down
Expand Up @@ -26,6 +26,9 @@ class Foo {
const aContext = Foo['aContext'];
const bContext = Foo['bContext'];

expect(aContext.access).not.toHaveProperty("set");
expect(bContext.access).not.toHaveProperty("set");

expect(aContext.access.has(Foo)).toBe(true);
expect(aContext.access.has({})).toBe(false);
expect(aContext.access.has(Object.create(Foo))).toBe(true);
Expand Down
@@ -1,5 +1,8 @@
let aContext;

function dec(fn, context) {
expect(fn.name).toEqual(context.name);
if (!aContext) aContext = context;
return function () {
return fn.call(this) + 1;
}
Expand All @@ -20,6 +23,8 @@ class Foo {

let foo = new Foo();

expect(aContext.access).not.toHaveProperty("set");

expect(foo.callA()).toBe(2);
foo.value = 123;
expect(foo.callA()).toBe(124);
@@ -1,5 +1,8 @@
let aContext;

function dec(fn, context) {
expect(fn.name).toEqual(context.name);
if (!aContext) aContext = context;
return function () {
return fn.call(this) + 1;
}
Expand All @@ -21,6 +24,8 @@ class Foo {

let foo = new Foo();

expect(aContext.access).not.toHaveProperty("set");

expect(foo.a()).toBe(2);
expect(foo.b()).toBe(2);
foo.value = 123;
Expand Down
@@ -1,5 +1,8 @@
let aContext;

function dec(fn, context) {
expect(fn.name).toEqual(context.name);
if (!aContext) aContext = context;
return function () {
return fn.call(this) + 1;
}
Expand All @@ -18,6 +21,8 @@ class Foo {
}
}

expect(aContext.access).not.toHaveProperty("set");

expect(Foo.callA()).toBe(2);
Foo.value = 123;
expect(Foo.callA()).toBe(124);
@@ -1,5 +1,8 @@
let aContext;

function dec(fn, context) {
expect(fn.name).toEqual(context.name);
if (!aContext) aContext = context;
return function () {
return fn.call(this) + 1;
}
Expand All @@ -19,6 +22,8 @@ class Foo {
}
}

expect(aContext.access).not.toHaveProperty("set");

expect(Foo.a()).toBe(2);
expect(Foo.b()).toBe(2);
Foo.value = 123;
Expand Down
Expand Up @@ -26,6 +26,8 @@ let foo = new Foo();

const aContext = foo['#aContext'];

expect(aContext.access).not.toHaveProperty("get");

expect(aContext.access.has(foo)).toBe(true);
expect(aContext.access.has({})).toBe(false);
expect(aContext.access.has(Object.create(foo))).toBe(false);
Expand Down
Expand Up @@ -28,6 +28,9 @@ let foo = new Foo();
const aContext = foo['aContext'];
const bContext = foo['bContext'];

expect(aContext.access).not.toHaveProperty("get");
expect(bContext.access).not.toHaveProperty("get");

expect(aContext.access.has(foo)).toBe(true);
expect(aContext.access.has({})).toBe(false);
expect(aContext.access.has(Object.create(foo))).toBe(true);
Expand Down
Expand Up @@ -24,6 +24,8 @@ class Foo {

const aContext = Foo['#aContext'];

expect(aContext.access).not.toHaveProperty("get");

expect(aContext.access.has(Foo)).toBe(true);
expect(aContext.access.has({})).toBe(false);
expect(aContext.access.has(Object.create(Foo))).toBe(false);
Expand Down
Expand Up @@ -26,6 +26,9 @@ class Foo {
const aContext = Foo['aContext'];
const bContext = Foo['bContext'];

expect(aContext.access).not.toHaveProperty("get");
expect(bContext.access).not.toHaveProperty("get");

expect(aContext.access.has(Foo)).toBe(true);
expect(aContext.access.has({})).toBe(false);
expect(aContext.access.has(Object.create(Foo))).toBe(true);
Expand Down