Conditionally conform to Combine.TopLevelDecoder#132
Conditionally conform to Combine.TopLevelDecoder#132MaxDesiatov merged 7 commits intoCoreOffice:masterfrom sharplet:top-level-decoder
Conversation
|
@sharplet thank you for the PR! Would you mind adding a short example of use with Combine to README and also a unit test (maybe based on the example code)? |
|
Added a test case and a new section to the README, let me know what you think! |
|
Looks like the swiftpm tests are failing on CI, possibly because the Swift compiler version isn't recent enough to know about the |
|
One other option would be to just drop |
This shouldn't be necessary because all versions of Mac Catalyst should have a version of Combine available.
|
|
|
Good news: |
| var name: String | ||
| } | ||
|
|
||
| @available(iOS 13.0, macOS 10.15.0, tvOS 13.0, watchOS 6.0, *) |
There was a problem hiding this comment.
I wonder if the solution would be to remove macOS 10.15.0 clause until that's released? We would avoid crashing on CI until then too. After all I wouldn't mind tagging a minor release just for enabling that later when Catalina is released and available on CI.
There was a problem hiding this comment.
Removing macOS from the availability annotation would instead result in a compiler error. The issue is that we successfully compile & link, but then attempt to run the tests on a platform where the framework doesn't exist and instead crash when the symbol can't be loaded.
If we want to exclude the test from macOS altogether, we could add #if !os(macOS) to the compiler directive at the top of the file. The tests won't be exercised on CI then, so it might still make sense to add a CI job that runs on Xcode 11 + iOS?
There was a problem hiding this comment.
The Xcode 11 job already tests on macOS, iOS and tvOS, so that should be fine.
MaxDesiatov
left a comment
There was a problem hiding this comment.
LGTM, thanks for the PR!
This enables use of `XMLDecoder` with Combine's `decode(_:decoder:)` operator by conditionally conforming to `TopLevelDecoder` when Combine is available. I explored doing the same for `TopLevelEncoder`, but `XMLEncoder` currently requires `rootKey:` to be specified at encoding time, so it's not possible to provide an implementation of `encode(_:)` without some design around how to dynamically determine the root key to use. * Conditionally conform to Combine.TopLevelDecoder * Add test and documentation for Combine integration * Remove macCatalyst from CombineTests This shouldn't be necessary because all versions of Mac Catalyst should have a version of Combine available. * Add CombineTests to the Xcode project * Disable CombineTests on macOS
This enables use of `XMLDecoder` with Combine's `decode(_:decoder:)` operator by conditionally conforming to `TopLevelDecoder` when Combine is available. I explored doing the same for `TopLevelEncoder`, but `XMLEncoder` currently requires `rootKey:` to be specified at encoding time, so it's not possible to provide an implementation of `encode(_:)` without some design around how to dynamically determine the root key to use. * Conditionally conform to Combine.TopLevelDecoder * Add test and documentation for Combine integration * Remove macCatalyst from CombineTests This shouldn't be necessary because all versions of Mac Catalyst should have a version of Combine available. * Add CombineTests to the Xcode project * Disable CombineTests on macOS
This enables use of
XMLDecoderwith Combine'sdecode(_:decoder:)operator by conditionally conforming toTopLevelDecoderwhen Combine is available.I explored doing the same for
TopLevelEncoder, butXMLEncodercurrently requiresrootKey:to be specified at encoding time, so it's not possible to provide an implementation ofencode(_:)without some design around how to dynamically determine the root key to use.