MessagePack.swift
MessagePack.swift copied to clipboard
Added Codable support
This is a lightweight support for Codable completed with tests.
A few features are missing:
- "codingPath" is missing in DecodingError.Context when decoding error is thrown
- "userInfo" is not propagated
- "superDecoder" and variants are not supported
So far in my daily work, we haven't encountered use cases that require the above functionality.
- The coding path is helpful to find the failing class if such an error is thrown.
- As fas as I know userInfo is just for the user to mark some decoders for whatever (It anyway not a big thing to implement this, it isn't with the coding path either, so...)
- I don't know about the super decoder either. It seems to be meant to, well, (de|en)code a super class in
init(from:)
andencode(to:)
, and because you can't callcontainer.encode(super)
, I guess. However, I think the implementation ofsuperDecoder
is finished with returning just a new_MessagePackDecoder
and for thesuperEncoder
it would be best to just nest a storage and give this storage to a new_MessagePackEncoder
. I think that's it.
Not using such a stack like storage (like JSONEncoder does) is brilliant! It also solves a lot of issues JSONEncoder has (like decode after an error was thrown, which didn't work in the beginning but works now), I don't know whether these were also you're thoughts, but just don't using a "stack storage" is in my opinion really 💎.
There is just one issue with your implementation, I think Data will not encode to .binary
, but to .array
(the way it does by default). I saw the test for that, but I think you're new test just didn't run neither on linux, nor on macOS (?). Of course, I may be totally mistaken, but I could bet I'm not (Have your run the test yourself?). The thing is, if a Data
instance is passed to a MessagePackEncoder
and encodeToMessagePack
is called, first a new _MessagePackEncoder
is constructed and then value.encode(to:)
is called. Not .encode(to:)
of Data is called. This does not encode Data
again (as String
, Bool
, Int
, etc. do), so in this case the encode
of one of the containers is never called with Data
. encode(to:)
of Data
request an unkeyed container and encodes a list of UInt8
values (I don't know how this implementation actually looks, but this is what it does in the end).
My approach to fix this is, not to call value.encode(to:)
, but create a internal function that encodes and does the stuff that is done in all three containers (By the way, I think there is quite massive code duplication in the implementation that could be avoided with more "shared" global functions), checking for Data, converting it to msgpack, or calling .encode(to:)
if the value is something else.
Anyway great implementation @andrew-eng!
What's the status of this?
@a2 what's blocking this PR? I can help if needed
What's the status of this? This would be a great addition!
Any news on this?