Fixed bugs with different enum formats, closes #3

README.md

@@ -46,6 +46,9 @@ Supported types and values for the deserialization:
  - `char` from a JavaScript string containing a single codepoint.
  - `String` from any JavaScript string.
  - Rust map (`HashMap`, `BTreeMap`, ...) from any JavaScript iterable producing `[key, value]` pairs (including but not limited to ES2015 `Map`).
+   > One exception being [internally tagged](https://serde.rs/enum-representations.html#internally-tagged) and [untagged](https://serde.rs/enum-representations.html#untagged) enums. These representations currently do not support deserializing map-like iterables. They only support deserialization from `Object` due to their special treatment in `serde`. 
+   >
+   > This restriction may be lifted at some point in the future if a `serde(with = ...)` attribute can define the expected Javascript representation of the variant, or if serde-rs/serde#1183 gets resolved.
  - `HashMap<String, _>` from any plain JavaScript object (`{ key1: value1, ... }`).
  - Rust sequence (tuple, `Vec`, `HashSet`, ...) from any JavaScript iterable (including but not limited to `Array`, ES2015 `Set`, etc.).
  - Rust byte buffer (see [`serde_bytes`](https://github.com/serde-rs/bytes)) from JavaScript `ArrayBuffer` or `Uint8Array`.

src/de.rs

@@ -236,9 +236,29 @@ impl<'de> de::Deserializer<'de> for Deserializer {
         } else if let Some(v) = self.value.as_bool() {
             visitor.visit_bool(v)
         } else if let Some(v) = self.value.as_f64() {
-            visitor.visit_f64(v)
+            if js_sys::Number::is_safe_integer(&self.value) {
+                visitor.visit_i64(v as i64)
+            } else {
+                visitor.visit_f64(v)
+            }
         } else if let Some(v) = self.value.as_string() {
             visitor.visit_string(v)
+        } else if js_sys::Array::is_array(&self.value) {
+            self.deserialize_seq(visitor)
+        } else if self.value.is_object() &&
+            // The only reason we want to support objects here is because serde uses
+            // `deserialize_any` for internally tagged enums
+            // (see https://github.com/cloudflare/serde-wasm-bindgen/pull/4#discussion_r352245020).
+            //
+            // We expect such enums to be represented via plain JS objects, so let's explicitly
+            // exclude Sets, Maps and any other iterables. These should be deserialized via concrete
+            // `deserialize_*` methods instead of us trying to guess the right target type.
+            //
+            // Hopefully we can rid of these hacks altogether once
+            // https://github.com/serde-rs/serde/issues/1183 is implemented / fixed on serde side.
+            !js_sys::Reflect::has(&self.value, &js_sys::Symbol::iterator()).unwrap_or(false)
+        {
+            self.deserialize_map(visitor)
         } else {
             self.invalid_type(visitor)
         }

tests/serde.rs

@@ -1,8 +1,12 @@
+use js_sys::Reflect;
 use serde::de::DeserializeOwned;
 use serde::{Deserialize, Serialize};
 use serde_wasm_bindgen::{from_value, to_value};
-use std::collections::{HashMap, HashSet};
 use std::fmt::Debug;
+use std::{
+    collections::{BTreeMap, HashMap, HashSet},
+    hash::Hash,
+};
 use wasm_bindgen::{JsCast, JsValue};
 use wasm_bindgen_test::*;
 
@@ -24,14 +28,48 @@ where
     test(value, value);
 }
 
+fn recurse_and_replace_maps(val: JsValue) -> Option<JsValue> {
+    if val.is_object() {
+        let obj = if js_sys::Map::instanceof(&val) {
+            js_sys::Object::from_entries(&js_sys::Array::from(&val)).unwrap()
+        } else {
+            val.unchecked_into()
+        };
+
+        for key in js_sys::Object::keys(&obj).values() {
+            let key = key.unwrap();
+            let val = Reflect::get(&obj, &key).unwrap();
+
+            if let Some(replacement) = recurse_and_replace_maps(val) {
+                Reflect::set(&obj, &key, &replacement).unwrap();
+            }
+        }
+
+        Some(JsValue::from(obj))
+    } else {
+        None
+    }
+}
+
 fn assert_json<R>(lhs_value: JsValue, rhs: R)
 where
     R: Serialize + DeserializeOwned + PartialEq + Debug,
 {
-    assert_eq!(
-        js_sys::JSON::stringify(&lhs_value).unwrap(),
-        serde_json::to_string(&rhs).unwrap(),
-    );
+    let lhs_value = if let Some(replacement) = recurse_and_replace_maps(lhs_value.clone()) {
+        replacement
+    } else {
+        lhs_value
+    };
+
+    if lhs_value.is_undefined() {
+        assert_eq!("null", serde_json::to_string(&rhs).unwrap())
+    } else {
+        assert_eq!(
+            js_sys::JSON::stringify(&lhs_value).unwrap(),
+            serde_json::to_string(&rhs).unwrap(),
+        );
+    }
+
     let restored_lhs: R = from_value(lhs_value.clone()).unwrap();
     assert_eq!(restored_lhs, rhs, "from_value from {:?}", lhs_value);
 }
@@ -78,20 +116,31 @@ macro_rules! test_float {
 macro_rules! test_enum {
     ($(# $attr:tt)* $name:ident) => {{
         #[derive(Debug, PartialEq, Serialize, Deserialize)]
-        enum $name<A, B> {
+        $(# $attr)*
+        enum $name<A, B> where A: Debug + Ord + Eq {
             Unit,
             Newtype(A),
             Tuple(A, B),
             Struct { a: A, b: B },
+            Map(BTreeMap<A, B>),
+            Seq { seq: Vec<B> } // internal tags cannot be directly embedded in arrays
         }
 
-        test_via_json($name::Unit::<(), ()>);
-        test_via_json($name::Newtype::<_, ()>("newtype content".to_string()));
+        test_via_json($name::Unit::<String, i32>);
+        test_via_json($name::Newtype::<_, i32>("newtype content".to_string()));
         test_via_json($name::Tuple("tuple content".to_string(), 42));
         test_via_json($name::Struct {
             a: "struct content".to_string(),
             b: 42,
         });
+        test_via_json($name::Map::<String, i32>(
+            vec![
+                ("a".to_string(), 12),
+                ("abc".to_string(), -1161),
+                ("b".to_string(), 64)
+            ].into_iter().collect()
+        ));
+        test_via_json($name::Seq::<i32, i32> { seq: vec![5, 63, 0, -62, 6] });
     }};
 }
 
@@ -214,10 +263,50 @@ fn enums() {
     test_enum! {
         ExternallyTagged
     }
-    test_enum! {
-        #[serde(tag = "tag")]
-        InternallyTagged
+
+    #[derive(Debug, PartialEq, Serialize, Deserialize)]
+    #[serde(tag = "tag")]
+    enum InternallyTagged<A, B>
+    where
+        A: Ord,
+    {
+        Unit,
+        Struct { a: A, b: B },
+        Sequence { seq: Vec<A> },
+        Map(BTreeMap<A, B>)
     }
+
+    test_via_json(InternallyTagged::Unit::<(), ()>);
+    test_via_json(InternallyTagged::Struct {
+        a: "struct content".to_string(),
+        b: 42,
+    });
+    test_via_json(InternallyTagged::Struct {
+        a: "struct content".to_string(),
+        b: 42.2,
+    });
+    test_via_json(InternallyTagged::<i32, ()>::Sequence {
+        seq: vec![12, 41, -11, -65, 961],
+    });
+
+
+    // Internal tags with maps are not properly deserialized from Map values due to the exclusion 
+    // of Iterables during deserialize_any(). They can be deserialized properly from plain objects
+    // so we can test that.
+    assert_eq!(
+        InternallyTagged::Map(
+            vec![
+                ("a".to_string(), 12), 
+                ("abc".to_string(), -1161), 
+                ("b".to_string(), 64)
+            ].into_iter().collect()
+        ), 
+        from_value::<InternallyTagged<String, i32>>(
+            js_sys::eval("({ 'tag': 'Map', 'a': 12, 'abc': -1161, 'b': 64 })").unwrap()
+        ).unwrap()
+    );
+
+
     test_enum! {
         #[serde(tag = "tag", content = "content")]
         AdjacentlyTagged