All committers have signed the CLA.

@seabaylea @tunniclm @EnriqueL8 @djones6 ^^^ please comment 🙂

This seems reasonable as things currently stand.

We were recently discussing how we are going to integrate other types of encoder, particularly with a view to how the Router would select from those available based on the headers provided by the client (eg Accept headers). It's worth mentioning that at some point we will probably want to generalize the approach here to apply to any type of encoder and to take Accept headers into account.

The approach in the proposal is also compatible with later extension to per-request encoders. For example, if it does eventually warrant the added complexity then it is possible to have an optional encoder closure on the request object that if not present will default back to the one defined on the Router.

A middleware could then set the request closure for given requests/routes to override the one on the Router as required.

One consideration for this approach is that it adds customJSONEncoder as a specific property to the Router.

As part of adding support for additional built-in encoders, it would be good if we provided a mechanism by which users could dynamically add support for new coders themselves. Would it make sense to set up a table of encoder creation closures (keyed against the associated Content-Type header), and provide access to add or replace the closures in the table?

What would the types of the table be? There isn't a general Encoder protocol that JSONEncoder etc conforms to, they are just classes. I guess we could type-erase to Any but that doesn't seem great.

Actually @seabaylea is right, currently Codable routing uses application/json as the Content-Type, without explicitly setting the charset. This could also lead to strange behaviour.

I'm all into this custom coding idea (we could support PropertyList coders as well), my only question is where do you want to store all the encoder / decoder lookup table?

@ianpartridge we could create new protocols and extend the coders with it by default.

What do you guys think about this?

We're definitely interested in other transports in the future, e.g. protocol buffers.

Creating our own protocol Encoder sounds doable 👍

Theres already the Encoder and Decoder protocols that the encoders/decoders conform to.

Yes but JSONEncoder doesn't conform to it, it has a separate _JSONEncoder class that does ¯\(ツ)/¯

Hmmm, it feels like it should... any idea why they've taken that approach? JSONEncoder already (technically) conforms to Encoder as it has the functions - could we add our own extension to JSONEncoder to make it actually conform to the protocol?

It doesn't have the functions... encoders need unkeyedContainer() etc. I guess they took this approach to stop people subclassing the actual encoder?

Does anything break if we also add a default (empty) implementation of those functions?

Yep, probably they just wanted to allow devs to subclass only the config part and extend the coders with extra info, that's why the internal _JSONEncoder class. I think we should just invent a new protcol, something that could contain the encoder, content type header, content type charset. Something like a serializer in Alamofire.

I think the ideal solution would be to use something like:

typealias EncoderBuilder = () -> Encoder
typealias DecoderBuilder = () -> Decoder

var encoders: [ContentType : EncoderBuilder ] = [:]
var decoders: [ContentType : DecoderBuilder ] = [:]

but the fact that Foundations JSONEncoder doesn't implement the standard protocols does cause a major issue (and we can't just extend JSONEncoder to conform to Encoder)

One option would be to create our own encoder and decoder protocol which is a subset of Encoder and Decoder, make JSONEncoder and JSONDecoder conform to them in extensions, and make the existing Encoder and Decoder protocols conform as well. e.g.:

protocol Enc {
    func encode<T>(_ value: T) throws -> Data where T : Encodable
}

extension JSONEncoder : Enc {
}

protocol Encoder : Enc {
}

The you can use any custom encoder without having to modify it, and you can also use the JSONEncoder/Decoder from Foundation.

AFAIK The natural flow of encoding for example is that the top layer JSONEncoder gets called on func encode<T: Encodable> that then calls the func encode(to encoder: Encoder) on T passing the _JSONEncoder. This function makes calls to specific containers needed from the encoder. I do not see how we could make the existing Encoder conform to this protocol Enc without breaking this flow...(I know we could give self) I think if a user wants to use a custom encoder, they have to make their top layer Encoder conform to this protocol. Maybe I'm missing something?

@seabaylea Yep, I was suggesting the same thing.
@EnriqueL8 yes, that's not an option, we'd eventually break things.

Tomorrow I'll try to work on this issue, I'm going to figure out the rest of the "puzzle". ;)

@seabaylea @EnriqueL8 @ianpartridge please check out the new proposal. I have a working implementation, but It needs to be tested properly and I have to write the docs as well.

Hey @tib, sorry for this late answer... I think it looks good. Go ahead and open a PR, so that we can clearly view the code changes required and their impact.