Remove misleading assertion in Spice type #25
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This removes the assert in
Spice
that was warning developers to callprepareIfNeeded()
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 callingprepareIfNeeded()
too early, which results in an incorrect key path for the child spice.Motivation and Context
Consider the following scenario.
Accessing
$appService
here triggers our assertion. That happens because initializingEnvironmentSpiceStore
installs the Combine observer onappService
before the spice store's parent–child relationship is set up. Developers then try to work around the assert by callingprepareIfNeeded()
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