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

Fix using initialSubscriptions with @AsyncOpen and @AutoOpen #8572

Merged
merged 1 commit into from
May 3, 2024

Conversation

tgoyne
Copy link
Member

@tgoyne tgoyne commented May 3, 2024

Moving the storage of initialSubscriptions from RealmConfiguration to SyncConfiguration means that AsyncOpen needs to explicitly copy them over to the newly created SyncConfiguration.

It turned out we had no tests for swiftui plus flexible sync, so I made the existing test suite run against both pbs and flx apps.

@tgoyne tgoyne self-assigned this May 3, 2024
@cla-bot cla-bot bot added the cla: yes label May 3, 2024
@tgoyne tgoyne added the no-jira-ticket Skip checking the PR title for Jira reference label May 3, 2024
Moving the storage of intialSubscriptions from RealmConfiguration to
SyncConfiguration means that AsyncOpen needs to explicitly copy them over to
the newly created SyncConfiguration.
@tgoyne tgoyne force-pushed the tg/swiftui-initial-subscriptions branch from d06feef to c44fa8a Compare May 3, 2024 17:35
Copy link
Member

@nirinchev nirinchev left a comment

Choose a reason for hiding this comment

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

Looks good - it's nice to see the tests generalized to also cover FLX. Only one question, which is not a problem with the fix, but rather something that caught my eye during the review.

Comment on lines +1540 to +1543
} else if let initialSubscriptions {
config = user.flexibleSyncConfiguration(cancelAsyncOpenOnNonFatalErrors: true,
initialSubscriptions: ObjectiveCSupport.convert(block: initialSubscriptions.callback),
rerunOnOpen: initialSubscriptions.rerunOnOpen)
Copy link
Member

Choose a reason for hiding this comment

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

Do we have to do something similar for clientResetMode and the before/after callbacks? I haven't looked much into the SwiftUI code, but it appears like something that's not obviously handled in this method.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, those are probably broken too. Will fix in a subsequent PR.

@tgoyne tgoyne merged commit 6f8d5e3 into master May 3, 2024
113 checks passed
@tgoyne tgoyne deleted the tg/swiftui-initial-subscriptions branch May 3, 2024 18:53
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes no-jira-ticket Skip checking the PR title for Jira reference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants