search

pointfreeco / composable-core-location

A library that bridges the Composable Architecture and Core Location.
MIT License
110 stars 56 forks source link

Swift concurrency #29

Open rcasula opened 2 years ago

rcasula commented 2 years ago

Update the Library to the new swift-concurrency standard. I was thinking if removing the composable architecture dependency makes sense.

This PR is still in draft because I have to:

Please let me know if this PR is any good!

stephencelis commented 2 years ago

@rcasula Thanks for looking into this! We definitely have plans to adopt this new style soon, but haven't had time to look into it. This looks good! Just a couple things we need to figure out in the future:

  1. Does the manager need to be protected in an actor? And what concurrency warnings show up when we turn on -Xfrontend -warn-concurrency -Xfrontend -enable-actor-data-race-checks?
  2. We'd probably want to coordinate this change with a library rename. Something like core-location-client or core-location-dependency.

Going to keep this open for now while we find the time to figure out those details. Thanks for getting this work started though!

3a4oT commented 1 year ago

Hey, what is the status of this PR? What needs to be done to wrap up the new release?

p4checo commented 1 year ago

For completeness' sake, following a discussion in this PF's slack thread, to ensure CLLocationManager is accessed from the main RunLoop an alternative solution could be to use a MainActorIsolated structure as introduced here.

It appears to ensure CLLocationManager's APIs are invoked on the Main RunLoop, so it has the benefit of avoiding the Task { @MainActor } and not requiring sprinkling @MainActor in all API closures, making it less verbose and possibly less error prone as well if more APIs are added in the future.

Seems like an interesting approach, but ideally someone with more experience in swift concurrency can chime in, or at least we have one more approach to consider 🤓

p4checo commented 1 year ago

@rcasula can this MR be marked as ready for review? What documentation updates are missing? Thanks!

3a4oT commented 1 year ago

friendly ping, can we please get an feedback?

3a4oT commented 4 months ago

Hello, any chances to proceed with it?

spiridonkopicl commented 2 days ago

Thank you @rcasula, you saved my day 🍻