Remove warnings @AutoOpen @AsyncOpen and @ObservedResults - #8068
Conversation
e8f218d to
af344a6
Compare
dianaafanador3
changed the title
af344a6 to
4380d99
Compare
| private var _state: AsyncOpenState = .connecting { | ||
| willSet { | ||
| objectWillChange.send() | ||
| DispatchQueue.main.async { |
There was a problem hiding this comment.
Why this dispatch to main here? Is this called from background threads?
There was a problem hiding this comment.
@tgoyne Because with @AutoOpen and @AsyncOpen we set the state directly from the view thread, (for example when there is no user logged when we change the state to waitingForUser), the warning is going to show even if we store it a different variable. We want to call objectWillChange.send() from the main thread to update the view, but we don't want to do it directly from the UI element.
There was a problem hiding this comment.
If the set is happening from within a call to update() I think we just don't want to trigger objectWillChange. If it's not, then we want to do the set immediately. Doing an async set causes problems for sendability and is just sort of spooky in general as it means we're temporarily in a weird inconsistent state.
| - note: It is recommend to use the configuration obtained from the user, using `user.configuration(partitionValue:)` | ||
| or `user.flexibleSyncConfiguration()` when adding a configuration to this property wrapper, and modify any option from it | ||
| if needed. |
There was a problem hiding this comment.
What other configuration could they even use? This only works for sync realms and that's the only way to construct a sync config.
There was a problem hiding this comment.
18d4e47 to
4e9d335
Compare
… this will cause undefined behavior` warnings for @AutOOpeN, @AsyncOpen, @ObservedResults and @ObservedSectionedResults. * Defer AutoOpen & AsyncOpen initialization, so it would not call `Realm.asyncOpen` more than once.
4e9d335 to
0c63f6d
Compare
| // we observe the changes in the app state to check for user changes, | ||
| // we store an internal state, so we could react to those changes (user login, user change, logout). | ||
| app.objectWillChange.sink { app in | ||
| switch self.appState { | ||
| case .loggedIn(let user): | ||
| if let newUser = app.currentUser, | ||
| user != newUser { | ||
| self.appState = .loggedIn(newUser) | ||
| self.asyncOpenState = .connecting | ||
| self.asyncOpenForUser(user) | ||
| } else if app.currentUser == nil { | ||
| self.asyncOpenState = .waitingForUser | ||
| self.appState = .loggedOut | ||
| } | ||
| case .loggedOut: | ||
| if let user = app.currentUser { | ||
| self.appState = .loggedIn(user) | ||
| self.asyncOpenState = .connecting | ||
| self.asyncOpenForUser(user) | ||
| } | ||
| } | ||
| }.store(in: &appCancellable) |
There was a problem hiding this comment.
I think this is a retain cycle. self retains appCancellable, appCancellable retains the closure, and the closure retains self. This probably needs a [weak self] capture.
There was a problem hiding this comment.
Changed this to avoid the retain cycle
| // we observe the changes in the app state to check for user changes, | ||
| // we store an internal state, so we could react to those changes (user login, user change, logout). | ||
| app.objectWillChange.sink { [weak self] app in | ||
| switch self?.appState { |
There was a problem hiding this comment.
Should do guard let self = self else { return } rather than checking for nil on every use.
… and @ObservedSectionedResults (#8068) * * Remove `Publishing changes from within view updates is not allowed, this will cause undefined behavior` warnings for @AutOOpeN, @AsyncOpen, @ObservedResults and @ObservedSectionedResults. * Defer AutoOpen & AsyncOpen initialization, so it would not call `Realm.asyncOpen` more than once.
3 participants
Removed warning from @AutOOpeN @AsyncOpen and @ObservedResults and @ObservedSectionResults
While working on removing the warning, I fixed an issue, where Realm.asyncOpen was been called twice when used in
@AutoOpenand@AsyncOpenonce when initialised and a second time if we injected a environment value, now this is done one when the wrapped value is called on the view, this is a similar approach to what we do in@ObservedResults.Also, I added a note to
@AutoOpenand@AsyncOpen, which encourage users to inject configurations which comes from a user instead of creating a RealmConfiguration and inject it to the property wrapper.