-
-
Notifications
You must be signed in to change notification settings - Fork 17
Allow keeping Atoms alive within a scope until it's deallocation #188
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -150,6 +150,11 @@ public struct AtomScope<Content: View>: View { | |||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
private extension AtomScope { | ||||||||||||||||||
@MainActor | ||||||||||||||||||
final class State: ObservableObject { | ||||||||||||||||||
let scopeState = ScopeState() | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
enum Inheritance { | ||||||||||||||||||
case environment(scopeID: ScopeID) | ||||||||||||||||||
case context(AtomViewContext) | ||||||||||||||||||
|
@@ -161,21 +166,23 @@ private extension AtomScope { | |||||||||||||||||
let overrideContainer: OverrideContainer | ||||||||||||||||||
let content: Content | ||||||||||||||||||
|
||||||||||||||||||
@State | ||||||||||||||||||
private var scopeToken = ScopeKey.Token() | ||||||||||||||||||
@Environment(\.store) | ||||||||||||||||||
private var environmentStore | ||||||||||||||||||
private var store | ||||||||||||||||||
|
||||||||||||||||||
@StateObject | ||||||||||||||||||
private var state = State() | ||||||||||||||||||
|
||||||||||||||||||
var body: some View { | ||||||||||||||||||
content.environment( | ||||||||||||||||||
\.store, | ||||||||||||||||||
environmentStore?.scoped( | ||||||||||||||||||
scopeID: scopeID, | ||||||||||||||||||
scopeKey: scopeToken.key, | ||||||||||||||||||
observers: observers, | ||||||||||||||||||
overrideContainer: overrideContainer | ||||||||||||||||||
) | ||||||||||||||||||
let scopedStore = store?.scoped( | ||||||||||||||||||
scopeID: scopeID, | ||||||||||||||||||
scopeKey: state.scopeState.token.key, | ||||||||||||||||||
observers: observers, | ||||||||||||||||||
overrideContainer: overrideContainer | ||||||||||||||||||
) | ||||||||||||||||||
|
||||||||||||||||||
scopedStore?.registerScope(state: state.scopeState) | ||||||||||||||||||
|
||||||||||||||||||
return content.environment(\.store, scopedStore) | ||||||||||||||||||
Comment on lines
+183
to
+185
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nitpick] Registering a scope via side effects directly in the SwiftUI View body can lead to unpredictable behaviors. Consider moving
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,4 @@ | ||
internal struct Graph: Equatable { | ||
var dependencies = [AtomKey: Set<AtomKey>]() | ||
var children = [AtomKey: Set<AtomKey>]() | ||
var subscribed = [SubscriberKey: Set<AtomKey>]() | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,4 @@ | ||
@MainActor | ||
internal struct Scope { | ||
let key: ScopeKey | ||
let observers: [Observer] | ||
let overrideContainer: OverrideContainer | ||
let ancestorScopeKeys: [ScopeID: ScopeKey] | ||
var atoms = Set<AtomKey>() | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
@usableFromInline | ||
@MainActor | ||
internal final class ScopeState { | ||
let token = ScopeKey.Token() | ||
|
||
#if !hasFeature(IsolatedDefaultValues) | ||
nonisolated init() {} | ||
#endif | ||
|
||
nonisolated(unsafe) var unregister: (@MainActor () -> Void)? | ||
|
||
// TODO: Use isolated synchronous deinit once it's available. | ||
// 0371-isolated-synchronous-deinit | ||
deinit { | ||
MainActor.performIsolated { [unregister] in | ||
unregister?() | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
internal struct ScopeValues { | ||
let key: ScopeKey | ||
let observers: [Observer] | ||
let overrideContainer: OverrideContainer | ||
let ancestorScopeKeys: [ScopeID: ScopeKey] | ||
} |
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.
The
store
property is missing its@Environment(\.store)
property wrapper, so it will never be injected and will always be nil. It should be declared as@Environment(\.store) private var store
.Copilot uses AI. Check for mistakes.