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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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< | ||
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; | ||
|
@@ -269,21 +277,6 @@ export default /* @no-mangle */ function applyDecs2311( | |
var isSetter = kind === PROP_KIND.SETTER; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aside: Commenting on Line 273 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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
|
@@ -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; | ||
|
@@ -378,7 +390,6 @@ export default /* @no-mangle */ function applyDecs2311( | |
}; | ||
|
||
if (!isSetter) { | ||
markExistingNonField(); | ||
_.get = isPrivate | ||
? isMethod | ||
? function (_this: any) { | ||
|
@@ -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) { | ||
|
@@ -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 { | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.