Add initializing lifecycle hook to AtomEffect #181
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new "initializing" lifecycle hook to the AtomEffect protocol that executes before an atom is used and initialized. Key changes include:
- Addition of a new InitializingEffect struct that runs its action during the "initializing" stage.
- Updates to AtomEffect’s documentation and default implementation, as well as changes in StoreContext to invoke the new hook.
- New and updated tests to verify that “initializing” is triggered appropriately in various effect scenarios, and updates to user-facing documentation.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/AtomsTests/Utilities/Utilities.swift | Added initializingCount and an initializing method in TestEffect for tracking the new hook’s invocation. |
| Tests/AtomsTests/Effect/UpdateEffectTests.swift | Updated tests to call initializing and verify it does not affect performedCount for update effects. |
| Tests/AtomsTests/Effect/ReleaseEffectTests.swift | Updated tests to call initializing and verify no side effect on performedCount for release effects. |
| Tests/AtomsTests/Effect/InitializingEffectTests.swift | Added tests ensuring that InitializingEffect successfully triggers its action during initialization. |
| Tests/AtomsTests/Effect/InitializeEffectTests.swift | Renamed test class and added initializing call to verify expected behavior with InitializeEffect. |
| Sources/Atoms/Effect/InitializingEffect.swift | New file introducing the InitializingEffect that runs an action before initialization. |
| Sources/Atoms/Effect/InitializeEffect.swift | Updated documentation to reflect the proper timing of the action (after initialization). |
| Sources/Atoms/Effect/AtomEffect.swift | Updated protocol and default implementations to incorporate the initializing lifecycle event. |
| Sources/Atoms/Core/StoreContext.swift | Modified execution order to call initializing before getting the atom’s value and then calling initialized. |
| Sources/Atoms/Atoms.docc/Atoms.md & README.md | Documentation updates to include the new InitializingEffect and clarify lifecycle descriptions. |
| let state = getState(of: atom, for: key) | ||
| let currentContext = AtomCurrentContext(store: self) | ||
|
|
||
| state.effect.initializing(context: currentContext) |
There was a problem hiding this comment.
Consider adding an inline comment here to clarify that the initializing hook is intentionally invoked before retrieving the atom's value to capture pre-initialization state.
| performedCount += 1 | ||
| } | ||
|
|
||
| effect.initializing(context: context) |
There was a problem hiding this comment.
[nitpick] It may help to add an inline comment explaining that the initializing hook is expected to be a no-op for update effects, which improves clarity of the test intent.
| performedCount += 1 | ||
| } | ||
|
|
||
| effect.initializing(context: context) |
There was a problem hiding this comment.
[nitpick] Consider adding a comment here to clarify why invoking initializing on release effects should not affect performedCount, aiding future test maintenance.
Pull Request Type
Issue for this PR
Link: #168