search

pointfreeco / swift-composable-architecture

A library for building applications in a consistent and understandable way, with composition, testing, and ergonomics in mind.
https://www.pointfree.co/collections/composable-architecture
MIT License
12.07k stars 1.41k forks source link

Make `Shared.wrappedValue` setter unavailable from async and introduce `Shared.withLock` #3136

Closed mbrandonw closed 2 months ago

mbrandonw commented 2 months ago

Currently we allow for unfettered mutations to @Shared, and while the underlying storage is locked and so it won't crash, that doesn't mean there isn't a possibility for a race/data corruption.

We were hoping that some of the new isolations tools from Swift may have made it more possible to make this safe and ergonomic at the same time, but we're not sure how that will work, and also it would only be Swift 6+ (and in Sept '24).

So, we think a safer thing to do for now is deprecate wrapperdValue { set } on @Shared and instead force withValue just like with our LockIsolated type.

There are more changes to make to this PR before merging, such as updating docs and tutorials, but wanted to get this out there early so people know it's coming.

OguzYuuksel commented 2 months ago

Could you also add the map function to the Shared and SharedReader modules? I tried to extend them quickly but couldn’t since initializers are not exposed. It would be great to have the possibility to import their initializers as SPI.

mbrandonw commented 2 months ago

Could you also add the map function to the Shared and SharedReader modules? I tried to extend them quickly but couldn’t since initializers are not exposed. It would be great to have the possibility to import their initializers as SPI.

Hi @OguzYuuksel, since this is asking for new functionality that is not related to this PR, would you mind starting a discussion instead? And can you please provide as much detail as possible, because it's not clear what you want from a map function that isn't already possible from @dynamicMemberLookup.

stephencelis commented 2 months ago

We weren't super happy about the ergonomic regression of mutating shared values, so we explored some middle ground. In the latest version of this PR, mutations are only forbidden from asynchronous contexts. This means you will still be free to mutate shared state from a reducer:

case .incrementButtonTapped:
  state.count += 1  // ✅

But you will get a warning with a helpful message when updating from an effect:

case .delayedIncrementButtonTapped:
  return .run { _ in
    @Shared(.count) var count
    count += 1  // ⚠️ Use '$shared.withValue' instead of mutating directly.
  }

And this warning will be an error in Swift 6 mode.