Skip to content

Remove misleading assertion in Spice type #25

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 3 commits into from
May 21, 2025

Conversation

simonbs
Copy link
Contributor

@simonbs simonbs commented May 21, 2025

This removes the assert in Spice that was warning developers to call prepareIfNeeded() too early. That assertion didn’t accurately reflect how nested spice stores initialize and led to incorrect user defaults key paths.

Description

Setting up an observer in the initializer of a nested SpiceStore will trigger the assert before the parent–child relationship is wired up. Developers will work around this by calling prepareIfNeeded() too early, which results in an incorrect key path for the child spice.

Motivation and Context

Consider the following scenario.

enum ServiceEnvironment: String, CaseIterable {
    case production
    case staging
}

final class AppSpiceStore: SpiceStore {
    @Spice(name: "🍃 Environment") var environment = EnvironmentSpiceStore()
}

final class EnvironmentSpiceStore: SpiceStore {
    @Spice(requiresRestart: true) var appService: ServiceEnvironment = .production

    private var cancellables: Set<AnyCancellable> = []

    init() {
        $appService
            .removeDuplicates()
            .dropFirst()
            .sink { _ in }
            .store(in: &cancellables)
    }
}

Accessing $appService here triggers our assertion. That happens because initializing EnvironmentSpiceStore installs the Combine observer on appService before the spice store's parent–child relationship is set up. Developers then try to work around the assert by calling prepareIfNeeded() in the child's initializer. This causes the child spice to use the wrong user defaults key, since its fully qualified key path isn’t known yet, ultimately making it appear that the value isn't properly saved because it's saved under an incorrect key.

Removing the assertion makes this pattern valid. The nested store will be prepared automatically when its parent is ready.

If we ever find ourselves needing this assert again, we should likely address it differently.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

@simonbs simonbs merged commit fe5cf1c into main May 21, 2025
9 checks passed
@simonbs simonbs deleted the bugfix/value-not-correct-on-launch branch May 21, 2025 07:45
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.

1 participant