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.
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
Add Moya.Response extension for Decodable types in Swift 4 #1118
Comments
Hmm, this is an interesting one. Not sure what to think of it yet. |
Thanks for the issue, @SD10 - this is a really good idea. Having standardized encoding/decoding by Swift standard library, I think it is natural for us to implement a basic map method that would cover this behavior. However, for now we don't have any Swift 4.0 branch and we are in the middle of Moya 9.0 release, so we might want to wait with that implementation. |
@sunshinejr thank you for the feedback. I would be willing to provide the implementation for this feature when Moya is ready to start the Swift 4 migration. Including writing the Rx/Reactive extensions, documentation, and testing. If there's anything else I can do the contribute in the meantime, please feel free to let me know. |
Thanks for taking the initiative @SD10! That sounds great, we'll update this issue once we cut a 9.0 release 😄 |
Please note that there is now Alamofire/Alamofire#2180 around which tracks support for the Please also note that the contributors have already confirmed that they have plans to support both |
@SD10 It looks interesting. I think I can do it :) |
Hey @devxoul, just making sure - do you think you will want to implement this one? If not we might want to add labels to seek for help 😉 |
@sunshinejr, oh yeah I'm currently working on it. I'll make a pull request in this week :) |
That's great! I assigned you to this task then 😉 |
#1335 🎉 |
Closing this in favor of #1335. Let's move the discussion there 🎉 |
Why do we close these issues if they get auto-closed after the PR is merged? Now, if for some reason the PR isn't merged, there's a big chance this issue gets overlooked and stays closed. |
Good point @BasThomas. I always thought it was to encourage discussion on the PR and not have it split across two places. Feel free to reopen this and maybe we can default to the policy you described Sent with GitHawk |
I can't remember this happening somewhere actually, and I feel like the auto-closing works best. But if I am in a minority here, I'd not want to change the way this is handled now. @Moya/contributors, what do you think? |
I think we should keep this issue open because...
Also this is will be automatically closed if #1335 is merged. |
We can close this now :) |
Hmm I have no idea why GitHub didn't close this issue automatically when #1335 is merged 🤔 |
Think it was confused because we closed an reopened manually 😛 |
Ah, that makes sense! |
It seems like handling response JSON is a highly debated topic due to there being so many JSON serialization libraries and no determined best practice.
However, with the addition of the
Codable
protocol to the Swift 4 Standard Library, I think this will become a widely used approach. I'm wondering if the Moya community would be willing to add some basic extensions toMoya.Response
to mapDecodable
types.Since
Codable
is part of the Standard Library I think it's a safe addition to Moya's core.A rough API would be:
Moya already supports mapping to JSON, Images, and Strings
Regardless, these are not difficult extensions to write on our own or provide in a community extension.
The text was updated successfully, but these errors were encountered: