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

Move binding warning suppression to dynamic member lookup #2740

Merged
merged 7 commits into from Jan 30, 2024

Conversation

stephencelis
Copy link
Member

In TCA it is considered a bug to send an action to a child feature when it is dismissed.

In SwiftUI, a focused text field binding will write to itself when the view it is presented in is dismissed.

So if you derive a TCA binding to a text field in a dismissable child feature, dismissing said feature will write to the text field and cause a warning to be emitted.

To work around this, we set a task local, and are currently setting it in debug-only methods that construct a binding from scratch. Creating bindings from scratch unfortunately leads to issues with animations, so we should avoid doing so.

Instead, we can leverage properties on store that bindings are derived from and suppress the warnings from there.

In TCA it is considered a bug to send an action to a child feature when
it is dismissed.

In SwiftUI, a focused text field binding will write to itself when the
view it is presented in is dismissed.

So if you derive a TCA binding to a text field in a dismissable child
feature, dismissing said feature will write to the text field and cause
a warning to be emitted.

To work around this, we set a task local, and are currently setting it
in debug-only methods that construct a binding from scratch. Creating
bindings from scratch unfortunately leads to issues with animations, so
we should avoid doing so.

Instead, we can leverage properties on store that bindings are derived
from and suppress the warnings from there.
}
@available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
@dynamicMemberLookup
public struct _StoreBindable_SwiftUI<State: ObservableState, Action, Value> {
Copy link
Member Author

Choose a reason for hiding this comment

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

While I think it might be possible to abstract these 3 "store binding" types, I'm not sure it's worth it or a good idea.

@stephencelis stephencelis merged commit efc79f9 into main Jan 30, 2024
7 checks passed
@stephencelis stephencelis deleted the fix-binding-warnings branch January 30, 2024 18:30
cgrindel-self-hosted-renovate bot added a commit to cgrindel/rules_swift_package_manager that referenced this pull request Jan 31, 2024
…ure to from: "1.7.1" (#887)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
|
[pointfreeco/swift-composable-architecture](https://togithub.com/pointfreeco/swift-composable-architecture)
| patch | `from: "1.7.0"` -> `from: "1.7.1"` |

---

### Release Notes

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

###
[`v1.7.1`](https://togithub.com/pointfreeco/swift-composable-architecture/releases/tag/1.7.1)

[Compare
Source](https://togithub.com/pointfreeco/swift-composable-architecture/compare/1.7.0...1.7.1)

#### What's Changed

- Fixed: The Composable Architecture's SwiftUI Binding helpers should
better preserve SwiftUI animation and transaction information
([pointfreeco/swift-composable-architecture#2740).
Previously, animations could be lost in certain situations.
- Fixed: `@ObservableState` now supports the `package` access modifier
(thanks [@&#8203;nnsnodnb](https://togithub.com/nnsnodnb),
[pointfreeco/swift-composable-architecture#2741).
- Deprecated: `Reducer.onChange(of:removeDuplicates:)` has been
deprecated
([pointfreeco/swift-composable-architecture#2744).
The `removeDuplicates` argument put strain on the compiler that could
cause reducers to not compile in time. Migration strategy: use an
explicit wrapper type that is `Equatable`, instead.
- Infrastructure: Migrating to 1.7 fixes (thanks
[@&#8203;acosmicflamingo](https://togithub.com/acosmicflamingo),
[pointfreeco/swift-composable-architecture#2732;
thanks [@&#8203;bricklife](https://togithub.com/bricklife),
[pointfreeco/swift-composable-architecture#2736;
thanks [@&#8203;zvona031](https://togithub.com/zvona031),
[pointfreeco/swift-composable-architecture#2738;
thanks [@&#8203;shgew](https://togithub.com/shgew);
[pointfreeco/swift-composable-architecture#2743);
README fix (thanks [@&#8203;yimajo](https://togithub.com/yimajo),
[pointfreeco/swift-composable-architecture#2734).

#### New Contributors

- [@&#8203;acosmicflamingo](https://togithub.com/acosmicflamingo) made
their first contribution in
[pointfreeco/swift-composable-architecture#2732
- [@&#8203;zvona031](https://togithub.com/zvona031) made their first
contribution in
[pointfreeco/swift-composable-architecture#2738
- [@&#8203;shgew](https://togithub.com/shgew) made their first
contribution in
[pointfreeco/swift-composable-architecture#2743
- [@&#8203;nnsnodnb](https://togithub.com/nnsnodnb) made their first
contribution in
[pointfreeco/swift-composable-architecture#2741

**Full Changelog**:
pointfreeco/swift-composable-architecture@1.7.0...1.7.1

</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://togithub.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://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMDAuMCIsInVwZGF0ZWRJblZlciI6IjM2LjEwMC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

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.

None yet

2 participants