- Notifications
You must be signed in to change notification settings - Fork46
Equatable sinks prototype#254
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:tomb/swiftui-testbed
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| } | ||
| } | ||
| // I guess this could be upstreamed to Blueprint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Oh yeah, I think I've seen this go by in POS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm just now noticing that SwiftUI'sFocusState is not Equatable. That makes it harder forViews to be Equatable, but I presume SwiftUI's internal comparison of non-Equatable View types handles it well.
| } | ||
| } | ||
| // I guess this could be upstreamed to Blueprint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm just now noticing that SwiftUI'sFocusState is not Equatable. That makes it harder forViews to be Equatable, but I presume SwiftUI's internal comparison of non-Equatable View types handles it well.
| letclose:(()->Void)? | ||
| typealiasOutput=Never | ||
| enumOutput{ | ||
| case close | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| structState{ | ||
| vartitle:String | ||
| varisAllCaps:Bool | ||
| lettrampoline=SinkTrampoline() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This seems like the best/only way to create a persistent identity that we can use to implementStableSink.== 👍
| letsink= context.makeSink(of: actionType) | ||
| sinks[ObjectIdentifier(actionType)]= sink | ||
| returnStableSink(trampoline:self) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I wonder if we could passsink intoStableSink.init here and implementStableSink.send using that reference, allowing us to deletesinks,bounce, anddestination. That would also avoid retaining anyWorkflow.Sink that is no longer referenced by anyStableSink
Maintainingsinks does add some safety (againstthis, I think?) by ensuring we use the latestWorkflow.Sink that theRenderContext has given us for a givenAction type. But not total safety, because we're not evicting fromsinks every sink that wasn't remade during the lastrender.
Is the need for that safety greater when the Rendering holdsStableSinks rather than functions closing overWorkflow.Sinks?
No description provided.