fix: Idempotent start#23
Conversation
| @ViewModel var model: AnyReactor = MyViewModel().eraseToAnyReactor() | ||
| ``` | ||
|
|
||
| `AnyReactor` does not start the wrapped reactor automatically. Start it explicitly: |
There was a problem hiding this comment.
This needs to go to BREAKING CHANGES section after.
There was a problem hiding this comment.
@plajdo can you please create an MD.file tracking the breaking changes for each version. Its easier to maintain if its part of the PR.
There was a problem hiding this comment.
I cannot push to this branch anymore, I will create a new PR from fork. If changelog is needed, please create one separately.
However: I checked that this change should not affect any of company's projects, so no further changes are necessary (calling start manually was a recommended pattern anyway).
There was a problem hiding this comment.
Pull request overview
This PR aims to make Reactor.start() safe to call multiple times by avoiding duplicate external subscriptions, and changes AnyReactor lifecycle so the wrapped reactor is not auto-started (requiring explicit start() by callers).
Changes:
- Added a subscription-existence guard to make
Reactor.start()behave idempotently. - Disabled
AnyReactorauto-start by removingbase.start()from its initializer and updating its docs. - Updated README to instruct explicitly calling
start()forAnyReactor.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Sources/GoodReactor/Core/Reactor.swift | Adds guidance on transform() side effects and guards start() based on existing subscriptions. |
| Sources/GoodReactor/Core/Erased/AnyReactor.swift | Removes implicit base.start() and updates lifecycle documentation for explicit starting. |
| README.md | Documents explicit start() usage for AnyReactor. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Cannot push to this branch anymore, let's follow the changes at #25. |
Added check whether any subscriptions for current Reactor are active. This makes
startfunction idempotent for callers, and effectively prevents creating duplicate subscriptions.Condition:
transformfunction must be free of side effects, as emptytransformwill be called multiple times, until first subscription is created for the Reactor.Disabled auto-start of AnyReactor type-erased type.
Previously this auto-start behaviour caused incorrectly stored Combine/non-combine subscriptions to external events from viewModels. Subscriptions were being stored under the wrapper type, which caused confusion and non-clear event routing.
Effects:
Call to
viewModel.start()now has to be explicit and will always be forwarded to correct concrete Reactor type.