Fix Date decoding with custom strategy #24
Conversation
* Added coerce to be able to switch between compatible representations where only the tag changes * Change coerce to explicitly pass the new tag. Fixed spacing issues * Renamed coerce to coerced * Apply suggestions from code review Incorporated fixes for argument prefix Co-Authored-By: buscarini <buscarini@gmail.com>
* XcodeGen * 4.2 * Bump sim * Update package names * Bump Xcode
* Add tests for playgrounds. * Update Makefile Co-Authored-By: mbrandonw <mbw234@gmail.com>
* Time * Money * typo * podspecs * update readme * comment about currencies * date conversions and more tests * positive examples in readme * clean up * more cleanup * Update README.md * wip * Update Tagged.podspec * clean up * clean u * Bump * Update Podspecs * run make xcodeproj * dependency * make xcodeproj * Bundle XcodeGen. Ignore Package.resolved
* Add workspace * Swift 5 * Bumps * Workaround for Swift bug. * Bumps * Bump 2 * Podspecs * Use Swift 5 Docker * Conditionally include development dependencies * Use PF-prefixed environment variable to scope * Update TaggedTests.swift * Revert "Update TaggedTests.swift" This reverts commit b5bea0b.
|
@buscarini Thanks for the PR! Looks like this branch doesn't merge cleanly, though. Can you reset your branch to |
|
I think now it should be fixed and can be merged. |
|
Looks great, thanks! |
| @@ -35,7 +35,12 @@ extension Tagged: Comparable where RawValue: Comparable { | |||
|
|
|||
| extension Tagged: Decodable where RawValue: Decodable { | |||
| public init(from decoder: Decoder) throws { | |||
| self.init(rawValue: try .init(from: decoder)) | |||
| do { | |||
| self.init(rawValue: try decoder.singleValueContainer().decode(RawValue.self)) | |||
There was a problem hiding this comment.
@buscarini Can you explain the difference between going through the container vs straight from decoder?
Im working on a related problem: with Pointfreeco's ArbitraryDecoder, if I go through the container I can customize whats being decoded (which I need e.g. for String based enums, which almost certainly fail from an arbitrary string).
But it would help if I understood the difference between the two approaches for your usecase, so that I dont introduce unexpected bugs.
Thanks
There was a problem hiding this comment.
Specifically, I consider going through the container to be the correct way (it gives the decoder a chance to do whatever it needs to do). So I wonder which usecase it breaks (why is the catch needed)
There was a problem hiding this comment.
This pull request is related to #23
I don't remember all the details right now, but I think the problem is that with the container there was an issue with decoding optionals, and using the init directly skipped custom date decoding strategies, so this was the only solution that fixed both issues. You can check this by downloading the repository, commenting each strategy and trying to pass the tests. You will see that in each case one test fails.
Used a try catch to be able to parse dates and optional types.