Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 35 additions & 11 deletions core/engine/src/object/operations.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use super::internal_methods::InternalMethodPropertyContext;
use crate::js_error;
use crate::value::JsVariant;
use crate::{
Context, JsResult, JsSymbol, JsValue,
Expand Down Expand Up @@ -887,23 +888,27 @@ impl JsObject {
value: JsValue,
context: &mut Context,
) -> JsResult<()> {
// 1. If the host is a web browser, then
// a. Perform ? HostEnsureCanAddPrivateElement(O).
if !self.extensible() {
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: It would be nice if we added the spec step to know where this came from. 😄

Suggested change
if !self.extensible() {
// 1. If O.[[Extensible]] is false, throw a TypeError exception.
//
// NOTE: From <https://tc39.es/proposal-nonextensible-applies-to-private/#sec-privatefieldadd>
if !self.extensible() {

return Err(js_error!(
TypeError: "cannot add private field to non-extensible class instance"
));
}

// 2. If the host is a web browser, then
// a. Perform ? HostEnsureCanAddPrivateElement(O).
context
.host_hooks()
.ensure_can_add_private_element(self, context)?;

// 2. Let entry be PrivateElementFind(O, P).
// 3. Let entry be PrivateElementFind(O, P).
let entry = self.private_element_find(name, false, false);

// 3. If entry is not empty, throw a TypeError exception.
// 4. If entry is not empty, throw a TypeError exception.
if entry.is_some() {
return Err(JsNativeError::typ()
.with_message("Private field already exists on prototype")
.into());
return Err(js_error!(TypeError: "private field already exists on prototype"));
}

// 4. Append PrivateElement { [[Key]]: P, [[Kind]]: field, [[Value]]: value } to O.[[PrivateElements]].
// 5. Append PrivateElement { [[Key]]: P, [[Kind]]: field, [[Value]]: value } to O.[[PrivateElements]].
self.borrow_mut()
.private_elements
.push((name.clone(), PrivateElement::Field(value)));
Expand Down Expand Up @@ -931,12 +936,25 @@ impl JsObject {
method,
PrivateElement::Method(_) | PrivateElement::Accessor { .. }
));

let (getter, setter) = if let PrivateElement::Accessor { getter, setter } = method {
(getter.is_some(), setter.is_some())
} else {
(false, false)
};

if !self.extensible() {
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: Same here:

Suggested change
if !self.extensible() {
// 2. If O.[[Extensible]] is false, throw a TypeError exception.
//
// NOTE: From <https://tc39.es/proposal-nonextensible-applies-to-private/#sec-privatemethodoraccessoradd>
if !self.extensible() {

return if getter || setter {
Err(js_error!(
TypeError: "cannot add private accessor to non-extensible class instance"
))
} else {
Err(js_error!(
TypeError: "cannot add private method to non-extensible class instance"
))
};
Comment on lines +947 to +955
Copy link
Member

Choose a reason for hiding this comment

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

Praise: I really like the error message Improvement 😄

}

// 2. If the host is a web browser, then
// a. Perform ? HostEnsureCanAddPrivateElement(O).
context
Expand All @@ -948,9 +966,15 @@ impl JsObject {

// 4. If entry is not empty, throw a TypeError exception.
if entry.is_some() {
return Err(JsNativeError::typ()
.with_message("Private method already exists on prototype")
.into());
return if getter || setter {
Err(js_error!(
TypeError: "private accessor already exists on class instance"
))
} else {
Err(js_error!(
TypeError: "private method already exists on class instance"
))
};
}

// 5. Append method to O.[[PrivateElements]].
Expand Down
Loading