-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Improve Boolean/Number/JsString consistency #1447
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
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 |
---|---|---|
|
@@ -467,13 +467,14 @@ extern "C" { | |
#[wasm_bindgen] | ||
extern "C" { | ||
#[wasm_bindgen(extends = Object)] | ||
#[derive(Clone, Debug)] | ||
#[derive(Clone)] | ||
pub type Boolean; | ||
|
||
/// The `Boolean()` constructor creates an object wrapper for a boolean value. | ||
/// | ||
/// [MDN documentation](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Boolean) | ||
#[wasm_bindgen(constructor)] | ||
#[deprecated(note = "recommended to use `Boolean::from` instead")] | ||
pub fn new(value: &JsValue) -> Boolean; | ||
|
||
/// The `valueOf()` method returns the primitive value of a `Boolean` object. | ||
|
@@ -483,6 +484,35 @@ extern "C" { | |
pub fn value_of(this: &Boolean) -> bool; | ||
} | ||
|
||
impl From<bool> for Boolean { | ||
#[inline] | ||
fn from(b: bool) -> Boolean { | ||
Boolean::unchecked_from_js(JsValue::from(b)) | ||
} | ||
} | ||
|
||
impl From<Boolean> for bool { | ||
#[inline] | ||
fn from(b: Boolean) -> bool { | ||
b.value_of() | ||
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 can probably be implemented in a more optimized way, but we can save that for later. |
||
} | ||
} | ||
|
||
impl PartialEq<bool> for Boolean { | ||
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. Shouldn't we impl Also, shouldn't all of these simple methods be 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. Indeed! |
||
#[inline] | ||
fn eq(&self, other: &bool) -> bool { | ||
self.value_of() == *other | ||
} | ||
} | ||
|
||
impl Eq for Boolean {} | ||
|
||
impl fmt::Debug for Boolean { | ||
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { | ||
self.value_of().fmt(f) | ||
} | ||
} | ||
|
||
// DataView | ||
#[wasm_bindgen] | ||
extern "C" { | ||
|
@@ -1406,7 +1436,7 @@ extern "C" { | |
#[wasm_bindgen] | ||
extern "C" { | ||
#[wasm_bindgen(extends = Object)] | ||
#[derive(Clone, Debug)] | ||
#[derive(Clone)] | ||
pub type Number; | ||
|
||
/// The Number.isFinite() method determines whether the passed value is a finite number. | ||
|
@@ -1441,6 +1471,7 @@ extern "C" { | |
/// | ||
/// [MDN documentation](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number) | ||
#[wasm_bindgen(constructor)] | ||
#[deprecated(note = "recommended to use `Number::from` instead")] | ||
pub fn new(value: &JsValue) -> Number; | ||
|
||
/// The Number.parseInt() method parses a string argument and returns an | ||
|
@@ -1500,6 +1531,38 @@ extern "C" { | |
pub fn value_of(this: &Number) -> f64; | ||
} | ||
|
||
macro_rules! number_from { | ||
($($x:ident)*) => ($( | ||
impl From<$x> for Number { | ||
#[inline] | ||
fn from(x: $x) -> Number { | ||
Number::unchecked_from_js(JsValue::from(x)) | ||
} | ||
} | ||
|
||
impl PartialEq<$x> for Number { | ||
#[inline] | ||
fn eq(&self, other: &$x) -> bool { | ||
self.value_of() == f64::from(*other) | ||
} | ||
} | ||
)*) | ||
} | ||
number_from!(i8 u8 i16 u16 i32 u32 f32 f64); | ||
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. Perhaps a silly suggestion, but why not use transitive generics instead of a custom macro? Something like: impl<T> From<T> for Number where f64: From<T> {
// ...
} The only downside it has is that it makes it impossible to implement custom 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. That's possible yeah, but I think I'd prefer to stick with the macro strategy to avoid blanket impls unnecessarily blocking out other impls. |
||
|
||
impl From<Number> for f64 { | ||
#[inline] | ||
fn from(n: Number) -> f64 { | ||
n.value_of() | ||
} | ||
} | ||
|
||
impl fmt::Debug for Number { | ||
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { | ||
self.value_of().fmt(f) | ||
} | ||
} | ||
|
||
// Date. | ||
#[wasm_bindgen] | ||
extern "C" { | ||
|
Uh oh!
There was an error while loading. Please reload this page.