Skip to content
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 asyncStream and asyncThrowingStream for Signal and SignalProducer #847

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

Marcocanc
Copy link
Member

@Marcocanc Marcocanc commented Nov 12, 2021

This is a proposal to add asyncStream and asyncThrowingStream in order to bridge into Swift Concurrency.
I find this especially useful for writing non-blocking async tests.

A couple of things I'd love to hear your thoughts on (apart from the usual review):

  • General thoughts on the idea
  • Where to place these extensions within the project and naming of files
  • Raising of CI platform to Xcode 11 (due to minimum Swift and testing target version requirements)
  • Lack of support for Swift Concurrency in Quick/Nimble (leading to tests written with XCTest)

Note: Mac Catalyst builds are failing to launch on pre-monterey macOS versions: There is a workaround described in the documents but probably not worth implementing in the CI. More info here.

Mac Catalyst apps that use Swift Concurrency may fail to launch on an operating system prior to macOS Monterey. (84393581)

@Marcocanc Marcocanc requested a review from andersio November 12, 2021 08:58
Copy link
Contributor

@mluisbrown mluisbrown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome 👏

Sources/Signal+SwiftConcurrency.swift Outdated Show resolved Hide resolved
Sources/SignalProducer+SwiftConcurrency.swift Outdated Show resolved Hide resolved
@Marcocanc
Copy link
Member Author

Marcocanc commented Nov 14, 2021

Interestingly the macCatalyst issue is known in Xcode 13.2 Beta
https://developer.apple.com/documentation/xcode-release-notes/xcode-13_2-release-notes
There's a workaround, but probably worth waiting (and hoping for a fix in 13.2 GA) rather than modifying the SDK in the CI.


@available(macOS 10.15, iOS 13, watchOS 6, tvOS 13, macCatalyst 13, *)
extension Signal {
public var asyncThrowingStream: AsyncThrowingStream<Value, Swift.Error> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use AsyncStream<Result<Value, Error>> to maintain the error type.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bump

switch event {
case .value(let value):
continuation.yield(value)
case .completed, .interrupted:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the interrupted event throw a swift CancellationError?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants