Support for Tuple #20
Conversation
src/Simple/JSON.purs
Outdated
| a <- readImpl $ get arr 0 | ||
| b <- readImpl $ get arr 1 | ||
| pure $ Tuple a b | ||
| l -> fail $ TypeMismatch "2 values" (show l <> " values") |
There was a problem hiding this comment.
Can't this provide more information?..
There was a problem hiding this comment.
What would you suggest?
src/Simple/JSON.purs
Outdated
| instance readForeignTuple :: (ReadForeign a, ReadForeign b) => ReadForeign (Tuple a b) where | ||
| readImpl = asTuple <=< readArray | ||
| where asTuple :: Array Foreign -> F (Tuple a b) | ||
| asTuple arr = case length arr of |
There was a problem hiding this comment.
I think this code would be simpler by pattern matching on arr
asTuple = case _ of
[a, b] -> ...
_ -> ...There was a problem hiding this comment.
Of course, thanks.
| ra <- readImpl a | ||
| rb <- readImpl b | ||
| pure $ Tuple ra rb | ||
| l -> fail $ TypeMismatch "2 values" (show (length l) <> " values") |
There was a problem hiding this comment.
Oh wait, I just realized now that you have a single array decoding. For reading, is there not a way we could somehow maybe read in the whole array and then map them back to the tuples? Might require some generics-rep code?
There was a problem hiding this comment.
Like (a /\ b /\ c /\ d) should be [a,b,c,d]?
There was a problem hiding this comment.
Sounds good. Separate PR?
There was a problem hiding this comment.
Realized that some actually do encode nested array pairs though. I think probably it'd be best to go with whatever you need with this for now.
There was a problem hiding this comment.
Oh, nested tuples don't serialise correctly, I'll try to fix it.
There was a problem hiding this comment.
I'm not sure I understood what you mean... I need ReadForeign Unit for encoding Tuple3 & friends and used a broken readImpl for now, but I guess Data.Foreign could provide that.
There was a problem hiding this comment.
You should never be using the value of Unit anywhere for reading or writing, it's only passed around like an opaque foreign import data declared type. You should always use an empty record for this.
There was a problem hiding this comment.
Unit's foreign representation should be null or undefined or something anyway, the value is absolutely meaningless because it's never supposed to be inspected or used.
There was a problem hiding this comment.
Is that what you mean? How would that be better?
In any case, a generic n-tuple serialisation as array probably won't need unit anyway, right?
There was a problem hiding this comment.
Well, as in no one will give you a stop element anyway. People expect (1 /\ 2 /\ 3) to become [1,2,3]
src/Simple/JSON.purs
Outdated
| @@ -182,7 +182,7 @@ instance writeForeignForeign :: WriteForeign Foreign where | |||
| writeImpl = id | |||
|
|
|||
| instance writeForeignUnit :: WriteForeign Unit where | |||
| writeImpl = toForeign | |||
| writeImpl = const $ writeImpl {} | |||
There was a problem hiding this comment.
Please don't do this, this is not supposed to work in any way.
src/Simple/JSON.purs
Outdated
| @@ -182,7 +182,7 @@ instance writeForeignForeign :: WriteForeign Foreign where | |||
| writeImpl = id | |||
|
|
|||
| instance writeForeignUnit :: WriteForeign Unit where | |||
There was a problem hiding this comment.
This instance should not exist at all, there is not valid value for Unit. This will also break in any non-JS backend like purerl.
There was a problem hiding this comment.
Then we'd need to use -generics-rep, right?
test/Main.purs
Outdated
| { "a": [1, "foo"] } | ||
| """ | ||
| it "works with nested Tuple" $ roundtrips (Proxy :: Proxy MyTestNestedTuple) """ | ||
| { "a": [1, ["bar", 4.2]] } |
There was a problem hiding this comment.
Test case here should be { "a": [1, "bar", 4.2] }
There was a problem hiding this comment.
ah, sorry, this comment is wrong anyway
There was a problem hiding this comment.
or there's really no sane way we can try to solve nested tuples and flatten them, it seems
There was a problem hiding this comment.
Should I just close this PR? Looks like there're easy workarounds, e.g, this should serialise as [x,y,z] properly:
data V3 = V3 Number Number Number
derive instance gV3 :: Generic V3 _
instance writeForeignV3 :: WriteForeign V3 where
writeImpl = genericEncode $ defaultOptions { unwrapSingleConstructors = true }
...
There was a problem hiding this comment.
Yeah, we could. If you need the simple pair tuple case we should put it in though.
test/Main.purs
Outdated
| { "a": [1, ["bar", 4.2]] } | ||
| """ | ||
| it "works with Tuple3" $ roundtrips (Proxy :: Proxy MyTestTuple3) """ | ||
| { "a": [1, ["bar", [4.2, {}]]] } |
There was a problem hiding this comment.
Test case here should be { "a": [1, "bar", 4.2] }
|
Closing this since we can't really represent tuples in a non-magic way... |
No description provided.