Skip to content

Resolve binding key path crash #1784

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

Merged
merged 1 commit into from
Jan 3, 2023

Conversation

barabashd
Copy link
Contributor

Details
In a nutshell: binding works differently from @swiftui.State binding and could lead to a crash.

@stephencelis
Copy link
Member

@barabashd Thanks for diving into the problem and your work on a solution!

Though the stack trace isn't super helpful, the crash you're encountering is that the row view computes for an element that no longer exists. This computation causes the array subscript to get called for an invalid index, so you get a "index out of range" fatal error. We're not quite sure why SwiftUI does this in this case, and we have considered it an Apple bug, but it's nice to see that there's a workaround by shuffling the domain around.

I'm going to merge this as-is and will make a few changes in a followup PR that fix the CI issue and an animation issue the original subscript intended to work around.

@stephencelis stephencelis merged commit fbe6847 into pointfreeco:main Jan 3, 2023
stephencelis added a commit that referenced this pull request Jan 3, 2023
This PR works on top of #1784 and:

  * Reduces the number of moving parts
  * Restores implicit animations by directly producing the binding from
    the observable object
  * Strongly retains the store in the binding to avoid losing writes
@stephencelis stephencelis mentioned this pull request Jan 3, 2023
stephencelis added a commit that referenced this pull request Jan 3, 2023
* Simplify/fix #1784

This PR works on top of #1784 and:

  * Reduces the number of moving parts
  * Restores implicit animations by directly producing the binding from
    the observable object
  * Strongly retains the store in the binding to avoid losing writes

* Move
stephencelis added a commit that referenced this pull request Jan 6, 2023
stephencelis added a commit that referenced this pull request Jan 6, 2023
stephencelis added a commit that referenced this pull request Jan 6, 2023
* Revert "Simplify/fix #1784 (#1785)"

This reverts commit 626c35c.

* Revert "resolve binding key path crash (#1784)"

This reverts commit fbe6847.
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.

2 participants