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.

Sign up for GitHub

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

Jump to bottom

Improve Boolean/Number/JsString consistency #1447

Merged
merged 1 commit into from
Apr 12, 2019
Merged

Improve Boolean/Number/JsString consistency #1447

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
67 changes: 65 additions & 2 deletions crates/js-sys/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,13 +467,14 @@ extern "C" {
#[wasm_bindgen]
extern "C" {
#[wasm_bindgen(extends = Object)]
#[derive(Clone, Debug)]
#[derive(Clone)]
alexcrichton marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand All @@ -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()
Copy link
Contributor

Choose a reason for hiding this comment

The 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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we impl Eq as well?

Also, shouldn't all of these simple methods be #[inline]?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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" {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The 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 From<SomeType> for Number but I guess we don't really want to support source types that are not convertible to f64 too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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" {
Expand Down