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

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

Merged
merged 18 commits into from
Jun 6, 2024

Conversation

mbrandonw
Copy link
Member

@mbrandonw mbrandonw commented Jun 3, 2024

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
Copy link

OguzYuuksel commented Jun 3, 2024

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
Copy link
Member Author

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.

public init(_ key: String) where Value == Bool {
fileprivate init(_ key: String) where Value == Bool {
Copy link
Member

Choose a reason for hiding this comment

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

While organizing DocC I noticed we had accidentally made a lot of APIs public that never should have been.

If we're worried about this being a breaking change, we can deprecate, but hopefully no one is using AppStorageKey.init over .appStorage...

@stephencelis
Copy link
Member

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.

@stephencelis stephencelis changed the title Add withValue to Shared, deprecate direct mutation. Make Shared.wrappedValue setter unavailable from async and introduce Shared.withLock Jun 6, 2024
@stephencelis stephencelis merged commit 1eeca17 into main Jun 6, 2024
7 checks passed
@stephencelis stephencelis deleted the shared-withvalue branch June 6, 2024 22:16
cgrindel-self-hosted-renovate bot added a commit to cgrindel/rules_swift_package_manager that referenced this pull request Jul 2, 2024
…ure to from: "1.11.2" (#1137)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
|
[pointfreeco/swift-composable-architecture](https://github.com/pointfreeco/swift-composable-architecture)
| minor | `from: "1.10.4"` -> `from: "1.11.2"` |

---

### Release Notes

<details>
<summary>pointfreeco/swift-composable-architecture
(pointfreeco/swift-composable-architecture)</summary>

###
[`v1.11.2`](https://github.com/pointfreeco/swift-composable-architecture/releases/tag/1.11.2)

[Compare
Source](https://github.com/pointfreeco/swift-composable-architecture/compare/1.11.1...1.11.2)

#### What's Changed

- Fixed: Avoid potential sendability warnings in Swift 6 mode
([pointfreeco/swift-composable-architecture#3167).
- Fixed: `PersistenceKeyDefault` no longer uses the loaded value as an
initial value (thanks
[@&#8203;fdzsergio](https://github.com/fdzsergio),
[pointfreeco/swift-composable-architecture#3174).
- Fixed: Address a potential deadlock by isolating `Shared.withLock` to
the main actor
([pointfreeco/swift-composable-architecture#3178).
- Fixed: Disfavor `Shared`'s optional dynamic member lookup
([pointfreeco/swift-composable-architecture#3170).
Note that this fix may be source breaking. See the [migration
guide](https://pointfreeco.github.io/swift-composable-architecture/main/documentation/composablearchitecture/migratingto1.11)
for more details.
- Fixed: Don't over-observe app storage mutations
([pointfreeco/swift-composable-architecture#3186).
- Fixed: `$shared.elements` is now stable based on identity, and
restricted to identified arrays
([pointfreeco/swift-composable-architecture#3187).
- Infrastructure: Drop Swift <5.9 support
([pointfreeco/swift-composable-architecture#3185).
Xcode 15 has been required for app submission since April, so we can
keep our Swift support in line with Apple's.
- Infrastructure: 1.11 migration guide fixes (thanks
[@&#8203;larryonoff](https://github.com/larryonoff),
[pointfreeco/swift-composable-architecture#3184);
tutorial typo fixes (thanks
[@&#8203;meltsplit](https://github.com/meltsplit),
[pointfreeco/swift-composable-architecture#3161).

#### New Contributors

- [@&#8203;meltsplit](https://github.com/meltsplit) made their first
contribution in
[pointfreeco/swift-composable-architecture#3161
- [@&#8203;fdzsergio](https://github.com/fdzsergio) made their first
contribution in
[pointfreeco/swift-composable-architecture#3174

**Full Changelog**:
pointfreeco/swift-composable-architecture@1.11.1...1.11.2

###
[`v1.11.1`](https://github.com/pointfreeco/swift-composable-architecture/releases/tag/1.11.1)

[Compare
Source](https://github.com/pointfreeco/swift-composable-architecture/compare/1.11.0...1.11.1)

#### What's Changed

- Fixed: Support swift-syntax from 600.0.0-latest
([pointfreeco/swift-composable-architecture#3160).
- Fixed: `Shared.withLock` now pass values by continuation
([pointfreeco/swift-composable-architecture#3154).
- Infrastructure: Clean up DocC and link to new migration guide in
README
([pointfreeco/swift-composable-architecture#3153);
SyncUp tutorial fixes (thanks
[@&#8203;dafurman](https://github.com/dafurman),
[pointfreeco/swift-composable-architecture#3139;
thanks [@&#8203;RuiAAPeres](https://github.com/RuiAAPeres),
[pointfreeco/swift-composable-architecture#3159);
note Swift bug in documentation
([pointfreeco/swift-composable-architecture#3157).

#### New Contributors

- [@&#8203;RuiAAPeres](https://github.com/RuiAAPeres) made their first
contribution in
[pointfreeco/swift-composable-architecture#3159

**Full Changelog**:
pointfreeco/swift-composable-architecture@1.11.0...1.11.1

###
[`v1.11.0`](https://github.com/pointfreeco/swift-composable-architecture/releases/tag/1.11.0)

[Compare
Source](https://github.com/pointfreeco/swift-composable-architecture/compare/1.10.4...1.11.0)

#### What's Changed

- Added: `Shared.withLock`, for mutating shared state from asynchronous
contexts
([pointfreeco/swift-composable-architecture#3136).
Direct mutations from asynchronous contexts is marked unavailable and
will be an error in Swift 6.
- Added: `SharedReader.constant`
([pointfreeco/swift-composable-architecture#3127).
- Added: `$store.scope` will now emit a warning when a dismiss action
doesn't `nil` out a child feature, suggesting a `Reducer.ifLet` (or
parent integration) is missing
([pointfreeco/swift-composable-architecture#3089).
- Deprecated: `Shared`'s optional dynamic member lookup overload has
been deprecated in favor of a `Binding.init` that unwraps optional
values
([pointfreeco/swift-composable-architecture#3145).
- Fixed: Avoid crash when using `.appStorage` with a `URL` value (thanks
[@&#8203;pwszebor](https://github.com/pwszebor),
[pointfreeco/swift-composable-architecture#3098).
- Fixed: Worked around a build failure when integrating with Tuist
([pointfreeco/swift-composable-architecture#3140).
- Infrastructure: Tutorial fixes
([pointfreeco/swift-composable-architecture#3076;
thanks [@&#8203;MartinMoizard](https://github.com/MartinMoizard),
[pointfreeco/swift-composable-architecture#3078;
[pointfreeco/swift-composable-architecture#3072;
thanks [@&#8203;hmhv](https://github.com/hmhv),
[pointfreeco/swift-composable-architecture#3091;
thanks [@&#8203;gibachan](https://github.com/gibachan),
[pointfreeco/swift-composable-architecture#3099;
thanks [@&#8203;btr-better](https://github.com/btr-better),
[pointfreeco/swift-composable-architecture#3107;
thanks [@&#8203;woxtu](https://github.com/woxtu),
[pointfreeco/swift-composable-architecture#3119,
[pointfreeco/swift-composable-architecture#3123;
[pointfreeco/swift-composable-architecture#3135;
[pointfreeco/swift-composable-architecture#3141;
[pointfreeco/swift-composable-architecture#3148);
DocC fixes
([pointfreeco/swift-composable-architecture#3085;
[pointfreeco/swift-composable-architecture#3087;
thanks [@&#8203;JOyo246](https://github.com/JOyo246),
[pointfreeco/swift-composable-architecture#3092;
thanks [@&#8203;leeari95](https://github.com/leeari95),
[pointfreeco/swift-composable-architecture#3110;
[pointfreeco/swift-composable-architecture#3138);
README fixes (thanks [@&#8203;Matt54](https://github.com/Matt54),
[pointfreeco/swift-composable-architecture#3129);
expose some navigation APIs with `@_spi(Internal)` (thanks
[@&#8203;Alex293](https://github.com/Alex293),
[pointfreeco/swift-composable-architecture#3097).

#### New Contributors

- [@&#8203;MartinMoizard](https://github.com/MartinMoizard) made their
first contribution in
[pointfreeco/swift-composable-architecture#3078
- [@&#8203;JOyo246](https://github.com/JOyo246) made their first
contribution in
[pointfreeco/swift-composable-architecture#3092
- [@&#8203;pwszebor](https://github.com/pwszebor) made their first
contribution in
[pointfreeco/swift-composable-architecture#3098
- [@&#8203;Alex293](https://github.com/Alex293) made their first
contribution in
[pointfreeco/swift-composable-architecture#3097
- [@&#8203;gibachan](https://github.com/gibachan) made their first
contribution in
[pointfreeco/swift-composable-architecture#3099
- [@&#8203;leeari95](https://github.com/leeari95) made their first
contribution in
[pointfreeco/swift-composable-architecture#3110
- [@&#8203;btr-better](https://github.com/btr-better) made their first
contribution in
[pointfreeco/swift-composable-architecture#3107
- [@&#8203;Matt54](https://github.com/Matt54) made their first
contribution in
[pointfreeco/swift-composable-architecture#3129

**Full Changelog**:
pointfreeco/swift-composable-architecture@1.10.4...1.11.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://github.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://github.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMDkuNCIsInVwZGF0ZWRJblZlciI6IjM2LjEwOS40IiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: Self-hosted Renovate Bot <361546+cgrindel-self-hosted-renovate[bot]@users.noreply.github.enterprise.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants