Conversation
WalkthroughThis PR performs a comprehensive architectural refactoring of the iOS app, transitioning from a ViewModel-based MVVM pattern to a new component-based architecture using the Observable framework. The old KotlinStateFlow and KotlinOptionalStateFlow property wrappers are removed and replaced with StateFlowObserver and OptionalStateFlowObserver components that leverage Swift's native Observable framework for SwiftUI state management. The Xcode project structure is reorganized with synchronized build file groups, build tools are simplified by removing the Danger plugin dependency, and the deployment target is updated to iOS 17 with Swift 6.2. Old View and ViewModel pairs across six screens (First, Second, Third, Login, Profile, Picker) are replaced with new Component and ComponentModel pairs. Navigation views are updated to use the new observer patterns, and test infrastructure is migrated from XCTest to the newer Testing framework. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (9)
iosApp/iosApp/App/RootView.swift (1)
15-22: Consider adding@MainActortoComponentHolderfor clarity.
ComponentHolderis accessed fromRootView(a SwiftUI View, implicitly@MainActor) via@State, which binds the instance to the main thread. While the currentnonisolateddeclaration is technically safe in Swift 6 because the object lifetime is guaranteed to be main-thread-bound, explicitly annotatingComponentHolderas@MainActorwould clarify the architectural intent and align with Swift 6 concurrency best practices.Suggested improvement
- nonisolated final class ComponentHolder<T> { + `@MainActor` + final class ComponentHolder<T> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iosApp/iosApp/App/RootView.swift` around lines 15 - 22, Annotate the ComponentHolder type with `@MainActor` to make its main-thread affinity explicit (update the ComponentHolder declaration to begin with `@MainActor`); then review/remove any nonisolated modifiers on its members if they conflict with the new isolation and ensure RootView’s use (via `@State` and access to componentHolder.lifecycle in onChange) compiles under the new actor annotation. This documents main-thread ownership and aligns ComponentHolder, RootView, and the lifecycle accesses (LifecycleRegistryExtKt.* calls) with Swift concurrency best practices.iosApp/iosApp/Views/Scenes/Second/SecondComponentModel.swift (1)
21-25: Consider MainActor dispatch for state updates.The
viewStateupdate occurs on an arbitrary executor. If this state will be exposed to SwiftUI in the future, wrap the assignment inMainActor.run.♻️ Proposed refactor
stateTask = Task { [weak self] in for await state in screen.viewState { - self?.viewState = state + await MainActor.run { self?.viewState = state } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iosApp/iosApp/Views/Scenes/Second/SecondComponentModel.swift` around lines 21 - 25, The Task assigned to stateTask is updating viewState off the main actor; wrap the assignment inside MainActor.run so UI-bound state is mutated on the main thread. Locate the async loop that iterates screen.viewState in the Task and replace the direct self?.viewState = state with a MainActor.run call that performs the assignment (keeping the [weak self] capture and nil-checking as before). Ensure any other UI-observed state mutations in this Task use MainActor.run as well.iosApp/iosApp/Views/Scenes/Second/SecondComponent.swift (1)
4-4: Consider making@Stateproperty private.Same as
ThirdComponent: SwiftLint flags that SwiftUI state properties should be private. Adding an explicit initializer improves encapsulation.♻️ Proposed refactor
struct SecondComponent<Model: SecondComponentModelProtocol>: View { - `@State` var model: Model + `@State` private var model: Model + + init(model: Model) { + _model = State(wrappedValue: model) + } var body: some View {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iosApp/iosApp/Views/Scenes/Second/SecondComponent.swift` at line 4, Make the `@State` property "model" in SecondComponent private and add an explicit initializer that accepts a Model parameter and assigns it to _model using State(initialValue:). Specifically, change the declaration to private `@State` var model: Model and add an init(model: Model) { self._model = State(initialValue: model) } so callers construct SecondComponent(model: ...) while keeping the state property encapsulated.iosApp/iosApp/Architecture/StateFlowObserver.swift (1)
16-19: Consider MainActor dispatch for UI-bound state updates.Same consideration as
OptionalStateFlowObserver: explicitly dispatching toMainActorensures thread-safe SwiftUI updates regardless of the source executor.♻️ Proposed refactor
task = Task { [weak self] in for await state in stateFlow { - self?.value = state + await MainActor.run { self?.value = state } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iosApp/iosApp/Architecture/StateFlowObserver.swift` around lines 16 - 19, StateFlowObserver's Task updates self?.value from an arbitrary executor—make the UI-bound update run on the MainActor to ensure thread-safe SwiftUI changes by awaiting or calling await MainActor.run { self?.value = state } (or mark the update closure `@MainActor`) inside the for-await loop that iterates stateFlow; update the Task block (symbol: task) and the for-await loop over stateFlow to perform the assignment to value on the MainActor so StateFlowObserver's UI state updates are always main-thread safe.iosApp/iosApp/Views/Scenes/Third/ThirdComponent.swift (1)
4-4: Consider making@Stateproperty private.SwiftLint flags that SwiftUI state properties should be private. While the current pattern works, marking
modelasprivatewith an explicit initializer improves encapsulation.♻️ Proposed refactor
struct ThirdComponent<Model: ThirdComponentModelProtocol>: View { - `@State` var model: Model + `@State` private var model: Model + + init(model: Model) { + _model = State(wrappedValue: model) + } var body: some View {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iosApp/iosApp/Views/Scenes/Third/ThirdComponent.swift` at line 4, The `@State` property 'model' in ThirdComponent should be made private to satisfy SwiftLint and improve encapsulation; update the declaration to 'private `@State` var model: Model' and add an explicit initializer on the ThirdComponent (e.g., init(model: Model)) that assigns the incoming model to the private `@State` so external callers can set it while keeping the state property private; ensure any references to 'model' inside ThirdComponent remain unchanged and external instantiation uses the new initializer.iosApp/iosApp/Architecture/OptionalStateFlowObserver.swift (1)
16-19: Add MainActor dispatch for thread-safe UI updates.SKIE's
SkieSwiftOptionalStateFlowis anAsyncSequencewith no thread restrictions—emissions are not guaranteed to arrive on the main thread. Since this is an@Observableclass for SwiftUI, dispatching toMainActorensures thread-safe state updates.♻️ Recommended refactor
task = Task { [weak self] in for await state in stateFlow { - self?.value = state + await MainActor.run { self?.value = state } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iosApp/iosApp/Architecture/OptionalStateFlowObserver.swift` around lines 16 - 19, The Task that iterates over stateFlow assigns self?.value off the main thread; wrap the UI update in a MainActor context so value is set on the main thread. Locate the Task closure that does "for await state in stateFlow { self?.value = state }" in OptionalStateFlowObserver (or where task is declared) and change the assignment to perform the update via MainActor (e.g., using MainActor.run or marking the closure `@MainActor`) so state emissions from SkieSwiftOptionalStateFlow are applied thread-safely to the Observable property.iosApp/iosApp/Views/Navigation/ProfileTabNavigationView.swift (1)
20-22: Avoid constructing component models inline inbody.Creating models in
bodycan repeatedly re-run initializers during render passes (notably for models that start Tasks). Prefer owning model initialization in a dedicated wrapper view/init path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iosApp/iosApp/Views/Navigation/ProfileTabNavigationView.swift` around lines 20 - 22, The ProfileComponentModel and ThirdComponentModel are being constructed inline inside the view's body (in the ProfileTabNavigationView switch), which can re-run their initializers on every render; move ownership of those models out of body by creating stored properties or wrapper subviews that initialize the models once (e.g., add let profileModel = ProfileComponentModel(...) and let thirdModel = ThirdComponentModel(...) initialized in ProfileTabNavigationView's initializer or create dedicated ProfileComponentContainer/ThirdComponentContainer that initialize and own the models), then pass those stored model instances to ProfileComponent(model:) and ThirdComponent(model:) from the body.iosApp/iosApp/Views/Scenes/Picker/PickerComponentModel.swift (1)
4-6: Separate picker identity from the localized title.
PickerItem.idis derived fromlocalized(), and the same string is sent back throughactions.onPick(item:). That makes both SwiftUI identity and selection semantics depend on the current translation; two distinct entries that localize to the same label become indistinguishable. SwiftUI expects identifiers to be stable and unique for a data item’s lifetime, so this likely wants a stable key from shared state plus a separate display title. (developer.apple.com)Also applies to: 23-26, 48-49
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iosApp/iosApp/Views/Scenes/Picker/PickerComponentModel.swift` around lines 4 - 6, PickerItem currently uses a localized string for its Identifiable id and is passed back via actions.onPick(item:), making identity and selection dependent on translations; change PickerItem to hold a stable unique identifier (e.g., let key: String or UUID) as the Identifiable id and add a separate computed/display property (e.g., var title: String { localized(...) }) for the localized label, then update places that construct PickerItem (lines referenced around 23-26) and the selection/action call sites (actions.onPick(item:) around 48-49) to use the stable id/key for identity and continue to pass or display the localized title separately.iosApp/iosApp/Views/Scenes/Login/LoginComponent.swift (1)
3-4: Use a plain stored property for the injected login model.This view never reads model state; it only forwards
onLoginClick(). Keeping the injected reference in@Statemakes it view-local storage, pins the initial instance to the view lifetime, and leaves the property externally mutable for no benefit. Aprivate let modelis the better ownership model here. Apple’s Observation examples pass injected models to subviews as plain stored properties, and SwiftUI guidance limits@Stateto private view-owned state. (developer.apple.com)♻️ Suggested change
struct LoginComponent<Model: LoginComponentModelProtocol>: View { - `@State` var model: Model + private let model: Model + + init(model: Model) { + self.model = model + } var body: some View { VStack(spacing: 10) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iosApp/iosApp/Views/Scenes/Login/LoginComponent.swift` around lines 3 - 4, The LoginComponent currently stores the injected Model as `@State` var model which makes it view-local mutable storage; change it to a plain stored property (e.g. private let model: Model) because the view never reads or mutates model and only forwards actions like model.onLoginClick(); update the initializer/signature of LoginComponent if needed so the model is passed in and stored as a non-@State let property and remove any `@State-related` semantics or usage tied to model.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@iosApp/iosApp/Views/Navigation/HomeTabNavigationView.swift`:
- Around line 38-42: The sheet builder currently constructs a new
PickerComponentModel inline (PickerComponentModel(instance.screen)) which starts
a long‑lived Task on init and can be re-created on every view body evaluation;
instead create and hold a stable model instance for the presented child (e.g.,
instantiate PickerComponentModel once per presented child and store it in a
SwiftUI-owned holder like `@StateObject` or a persistent property) and pass that
stored instance into PickerComponent; update the code paths that match the
onEnum(of: child) .picker case to use the stable model rather than calling
PickerComponentModel(...) inline so you avoid duplicate collectors and leaks.
In `@iosApp/iosApp/Views/Scenes/First/FirstComponent.swift`:
- Line 4: The `@State` property model on FirstComponent should be made private to
encapsulate internal SwiftUI state; update the declaration of the property named
model (of type Model) inside the FirstComponent view/struct to be private (e.g.,
change "@State var model: Model" to a private `@State` declaration) so it follows
the private_swiftui_state rule and prevents external access.
In `@iosApp/iosApp/Views/Scenes/Picker/PickerComponent.swift`:
- Line 4: The `@State` property for the component model (model: Model) is not
private; change its declaration to use private access control so SwiftUI state
is encapsulated (i.e., make the `@State` property private for the Model instance
used by the PickerComponent). Locate the declaration of model (the `@State` var
model: Model) and update it to be private, keeping the same type and semantics
so other code paths using the PickerComponent remain unchanged.
In `@iosApp/iosApp/Views/Scenes/Profile/ProfileComponent.swift`:
- Line 4: The `@State` property "model" in ProfileComponent (declared as "@State
var model: Model") must be made private to prevent external mutation; change its
declaration to mark it private (e.g., "private `@State` var model: Model") so the
SwiftUI state is only owned by the view and not accessible from other types.
In `@iosApp/iosApp/Views/Scenes/Second/SecondComponentModel.swift`:
- Line 13: SecondComponentModel has a private viewState property that is
subscribed and updated but never read; remove the unused overhead by either
deleting the private viewState and any async Task/subscription that updates it,
or make viewState public (or `@Published`) and wire it into the view so it is
actually consumed. Locate viewState and the async update logic inside
SecondComponentModel, then either (a) remove the viewState declaration and
remove/stop any Task or Combine subscription that writes to it, or (b) change
its access to public (or `@Published` public) and update the view code to
reference SecondComponentModel.viewState so the subscription is used. Ensure no
dangling tasks/subscriptions remain after the change.
In `@iosApp/iosAppTests/AppTests.swift`:
- Around line 4-7: The test example() in AppTests.swift currently uses a
tautological `#expect`(true); replace it with a deterministic assertion that
verifies real app behavior: call the code path you want to protect (e.g.,
instantiate the app or view controller, trigger a state change or navigation, or
call a method) and assert its observable outcome using XCTest assertions
(XCTAssertEqual/XCTAssertTrue/XCTAssertNotNil) — update the example() test to
initialize the relevant class or view controller, perform the action, and assert
the expected state/result so the test can catch regressions.
---
Nitpick comments:
In `@iosApp/iosApp/App/RootView.swift`:
- Around line 15-22: Annotate the ComponentHolder type with `@MainActor` to make
its main-thread affinity explicit (update the ComponentHolder declaration to
begin with `@MainActor`); then review/remove any nonisolated modifiers on its
members if they conflict with the new isolation and ensure RootView’s use (via
`@State` and access to componentHolder.lifecycle in onChange) compiles under the
new actor annotation. This documents main-thread ownership and aligns
ComponentHolder, RootView, and the lifecycle accesses (LifecycleRegistryExtKt.*
calls) with Swift concurrency best practices.
In `@iosApp/iosApp/Architecture/OptionalStateFlowObserver.swift`:
- Around line 16-19: The Task that iterates over stateFlow assigns self?.value
off the main thread; wrap the UI update in a MainActor context so value is set
on the main thread. Locate the Task closure that does "for await state in
stateFlow { self?.value = state }" in OptionalStateFlowObserver (or where task
is declared) and change the assignment to perform the update via MainActor
(e.g., using MainActor.run or marking the closure `@MainActor`) so state emissions
from SkieSwiftOptionalStateFlow are applied thread-safely to the Observable
property.
In `@iosApp/iosApp/Architecture/StateFlowObserver.swift`:
- Around line 16-19: StateFlowObserver's Task updates self?.value from an
arbitrary executor—make the UI-bound update run on the MainActor to ensure
thread-safe SwiftUI changes by awaiting or calling await MainActor.run {
self?.value = state } (or mark the update closure `@MainActor`) inside the
for-await loop that iterates stateFlow; update the Task block (symbol: task) and
the for-await loop over stateFlow to perform the assignment to value on the
MainActor so StateFlowObserver's UI state updates are always main-thread safe.
In `@iosApp/iosApp/Views/Navigation/ProfileTabNavigationView.swift`:
- Around line 20-22: The ProfileComponentModel and ThirdComponentModel are being
constructed inline inside the view's body (in the ProfileTabNavigationView
switch), which can re-run their initializers on every render; move ownership of
those models out of body by creating stored properties or wrapper subviews that
initialize the models once (e.g., add let profileModel =
ProfileComponentModel(...) and let thirdModel = ThirdComponentModel(...)
initialized in ProfileTabNavigationView's initializer or create dedicated
ProfileComponentContainer/ThirdComponentContainer that initialize and own the
models), then pass those stored model instances to ProfileComponent(model:) and
ThirdComponent(model:) from the body.
In `@iosApp/iosApp/Views/Scenes/Login/LoginComponent.swift`:
- Around line 3-4: The LoginComponent currently stores the injected Model as
`@State` var model which makes it view-local mutable storage; change it to a plain
stored property (e.g. private let model: Model) because the view never reads or
mutates model and only forwards actions like model.onLoginClick(); update the
initializer/signature of LoginComponent if needed so the model is passed in and
stored as a non-@State let property and remove any `@State-related` semantics or
usage tied to model.
In `@iosApp/iosApp/Views/Scenes/Picker/PickerComponentModel.swift`:
- Around line 4-6: PickerItem currently uses a localized string for its
Identifiable id and is passed back via actions.onPick(item:), making identity
and selection dependent on translations; change PickerItem to hold a stable
unique identifier (e.g., let key: String or UUID) as the Identifiable id and add
a separate computed/display property (e.g., var title: String { localized(...)
}) for the localized label, then update places that construct PickerItem (lines
referenced around 23-26) and the selection/action call sites
(actions.onPick(item:) around 48-49) to use the stable id/key for identity and
continue to pass or display the localized title separately.
In `@iosApp/iosApp/Views/Scenes/Second/SecondComponent.swift`:
- Line 4: Make the `@State` property "model" in SecondComponent private and add an
explicit initializer that accepts a Model parameter and assigns it to _model
using State(initialValue:). Specifically, change the declaration to private
`@State` var model: Model and add an init(model: Model) { self._model =
State(initialValue: model) } so callers construct SecondComponent(model: ...)
while keeping the state property encapsulated.
In `@iosApp/iosApp/Views/Scenes/Second/SecondComponentModel.swift`:
- Around line 21-25: The Task assigned to stateTask is updating viewState off
the main actor; wrap the assignment inside MainActor.run so UI-bound state is
mutated on the main thread. Locate the async loop that iterates screen.viewState
in the Task and replace the direct self?.viewState = state with a MainActor.run
call that performs the assignment (keeping the [weak self] capture and
nil-checking as before). Ensure any other UI-observed state mutations in this
Task use MainActor.run as well.
In `@iosApp/iosApp/Views/Scenes/Third/ThirdComponent.swift`:
- Line 4: The `@State` property 'model' in ThirdComponent should be made private
to satisfy SwiftLint and improve encapsulation; update the declaration to
'private `@State` var model: Model' and add an explicit initializer on the
ThirdComponent (e.g., init(model: Model)) that assigns the incoming model to the
private `@State` so external callers can set it while keeping the state property
private; ensure any references to 'model' inside ThirdComponent remain unchanged
and external instantiation uses the new initializer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d6b1b35c-4e95-4ea5-8e2d-37178b2af634
⛔ Files ignored due to path filters (1)
iosApp/Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (51)
iosApp/.gitignoreiosApp/.swiftlint.ymliosApp/DangerfileiosApp/GemfileiosApp/fastlane/FastfileiosApp/iosApp.xcodeproj/project.pbxprojiosApp/iosApp.xcodeproj/xcshareddata/xcschemes/Build KMP.xcschemeiosApp/iosApp.xcodeproj/xcshareddata/xcschemes/iosApp.xcschemeiosApp/iosApp/App/AppDelegate.swiftiosApp/iosApp/App/RootView.swiftiosApp/iosApp/Architecture/AlertModel.swiftiosApp/iosApp/Architecture/ComponentHolder.swiftiosApp/iosApp/Architecture/KotlinOptionalStateFlow.swiftiosApp/iosApp/Architecture/KotlinStateFlow.swiftiosApp/iosApp/Architecture/OptionalStateFlowObserver.swiftiosApp/iosApp/Architecture/StateFlowObserver.swiftiosApp/iosApp/Architecture/View+EventsEffect.swiftiosApp/iosApp/Preview Content/Preview Assets.xcassets/Contents.jsoniosApp/iosApp/Views/Navigation/HomeTabNavigationView.swiftiosApp/iosApp/Views/Navigation/Interop/DecomposeNavigationStack.swiftiosApp/iosApp/Views/Navigation/Interop/TabContentView.swiftiosApp/iosApp/Views/Navigation/ProfileTabNavigationView.swiftiosApp/iosApp/Views/Navigation/RootNavigationView.swiftiosApp/iosApp/Views/Navigation/SignedInNavigationView.swiftiosApp/iosApp/Views/Scenes/First/FirstComponent.swiftiosApp/iosApp/Views/Scenes/First/FirstComponentModel.swiftiosApp/iosApp/Views/Scenes/Login/LoginComponent.swiftiosApp/iosApp/Views/Scenes/Login/LoginComponentModel.swiftiosApp/iosApp/Views/Scenes/Picker/PickerComponent.swiftiosApp/iosApp/Views/Scenes/Picker/PickerComponentModel.swiftiosApp/iosApp/Views/Scenes/Profile/ProfileComponent.swiftiosApp/iosApp/Views/Scenes/Profile/ProfileComponentModel.swiftiosApp/iosApp/Views/Scenes/Second/SecondComponent.swiftiosApp/iosApp/Views/Scenes/Second/SecondComponentModel.swiftiosApp/iosApp/Views/Scenes/Third/ThirdComponent.swiftiosApp/iosApp/Views/Scenes/Third/ThirdComponentModel.swiftiosApp/iosApp/Views/Screen/First/FirstView.swiftiosApp/iosApp/Views/Screen/First/FirstViewModel.swiftiosApp/iosApp/Views/Screen/Login/LoginView.swiftiosApp/iosApp/Views/Screen/Login/LoginViewModel.swiftiosApp/iosApp/Views/Screen/Picker/PickerView.swiftiosApp/iosApp/Views/Screen/Picker/PickerViewModel.swiftiosApp/iosApp/Views/Screen/Profile/ProfileViewModel.swiftiosApp/iosApp/Views/Screen/Second/SecondView.swiftiosApp/iosApp/Views/Screen/Second/SecondViewModel.swiftiosApp/iosApp/Views/Screen/Third/ThirdView.swiftiosApp/iosApp/Views/Screen/Third/ThirdViewModel.swiftiosApp/iosAppTests/AppTests.swiftiosApp/iosAppTests/iosAppTests.swiftiosApp/iosAppUITests/iosAppUITestsLaunchTests.swiftiosApp/shared/KMP/Package.swift
💤 Files with no reviewable changes (19)
- iosApp/iosApp/Views/Screen/Picker/PickerView.swift
- iosApp/iosApp/Views/Screen/Third/ThirdView.swift
- iosApp/iosApp/Views/Screen/Profile/ProfileViewModel.swift
- iosApp/iosApp/Views/Screen/Login/LoginView.swift
- iosApp/iosApp/Views/Screen/First/FirstViewModel.swift
- iosApp/iosApp/Architecture/View+EventsEffect.swift
- iosApp/Dangerfile
- iosApp/iosApp/Architecture/KotlinStateFlow.swift
- iosApp/iosApp/Views/Screen/Login/LoginViewModel.swift
- iosApp/Gemfile
- iosApp/iosApp/Views/Screen/Second/SecondViewModel.swift
- iosApp/iosAppUITests/iosAppUITestsLaunchTests.swift
- iosApp/iosApp/Views/Screen/Third/ThirdViewModel.swift
- iosApp/iosApp/Preview Content/Preview Assets.xcassets/Contents.json
- iosApp/iosApp/Views/Screen/Picker/PickerViewModel.swift
- iosApp/iosApp/Views/Screen/Second/SecondView.swift
- iosApp/iosApp/Architecture/KotlinOptionalStateFlow.swift
- iosApp/iosApp/Views/Screen/First/FirstView.swift
- iosApp/iosAppTests/iosAppTests.swift
| if let child = sheet.value.child?.instance { | ||
| switch onEnum(of: child) { | ||
| case .picker(let instance): | ||
| PickerView(PickerViewModel(instance.screen)) | ||
| case let .picker(instance): | ||
| PickerComponent(model: PickerComponentModel(instance.screen)) | ||
| .presentationDetents(.init([.medium])) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and read the HomeTabNavigationView.swift file, focusing on lines 38-42 and context
find . -name "HomeTabNavigationView.swift" -type fRepository: futuredapp/kmp-futured-template
Length of output: 135
🏁 Script executed:
# Find and read the PickerComponentModel.swift file, focusing on lines 34-46
find . -name "PickerComponentModel.swift" -type fRepository: futuredapp/kmp-futured-template
Length of output: 137
🏁 Script executed:
# Read HomeTabNavigationView.swift to verify lines 38-42
cat -n ./iosApp/iosApp/Views/Navigation/HomeTabNavigationView.swiftRepository: futuredapp/kmp-futured-template
Length of output: 1968
🏁 Script executed:
# Read PickerComponentModel.swift to verify lines 34-46
cat -n ./iosApp/iosApp/Views/Scenes/Picker/PickerComponentModel.swiftRepository: futuredapp/kmp-futured-template
Length of output: 2351
Don't instantiate PickerComponentModel inside the sheet builder.
PickerComponentModel.init(_:) immediately starts a long-lived Task that collects from screen.viewState and only cancels it in deinit (PickerComponentModel.swift, lines 34-46). Creating the model inline here means every sheet content evaluation can spin up a fresh collector before the previous instance is released. Create the picker model once per presented child and pass that stable instance into PickerComponent. SwiftUI view bodies are reevaluated on state changes, so side-effectful initialization in these expressions risks duplicate task creation and resource leaks.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@iosApp/iosApp/Views/Navigation/HomeTabNavigationView.swift` around lines 38 -
42, The sheet builder currently constructs a new PickerComponentModel inline
(PickerComponentModel(instance.screen)) which starts a long‑lived Task on init
and can be re-created on every view body evaluation; instead create and hold a
stable model instance for the presented child (e.g., instantiate
PickerComponentModel once per presented child and store it in a SwiftUI-owned
holder like `@StateObject` or a persistent property) and pass that stored instance
into PickerComponent; update the code paths that match the onEnum(of: child)
.picker case to use the stable model rather than calling
PickerComponentModel(...) inline so you avoid duplicate collectors and leaks.
| import SwiftUI | ||
|
|
||
| struct FirstComponent<Model: FirstComponentModelProtocol>: View { | ||
| @State var model: Model |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "FirstComponent.swift" -type fRepository: futuredapp/kmp-futured-template
Length of output: 130
🏁 Script executed:
# Look for files in iosApp directory
fd -t f "FirstComponent" .Repository: futuredapp/kmp-futured-template
Length of output: 295
🏁 Script executed:
# Also check what Swift files exist in the specified path
fd -t f "\.swift$" "iosApp" | head -20Repository: futuredapp/kmp-futured-template
Length of output: 1117
🏁 Script executed:
cat -n ./iosApp/iosApp/Views/Scenes/First/FirstComponent.swift | head -30Repository: futuredapp/kmp-futured-template
Length of output: 1176
🏁 Script executed:
# Also check if there are any SwiftLint configuration files
find . -name ".swiftlint.yml" -o -name ".swiftlint.yaml" | head -5Repository: futuredapp/kmp-futured-template
Length of output: 98
🏁 Script executed:
cat ./iosApp/.swiftlint.ymlRepository: futuredapp/kmp-futured-template
Length of output: 2402
🏁 Script executed:
# Check for any other references to this model property outside the struct
rg "FirstComponent.*model" iosApp/iosApp/Views/Scenes/First/ -A 2 -B 2Repository: futuredapp/kmp-futured-template
Length of output: 442
🌐 Web query:
SwiftLint private_swiftui_state rule
💡 Result:
SwiftLint’s private_swiftui_state rule enforces that SwiftUI state properties are declared private (e.g. @State private var …, @StateObject private var …) in SwiftUI types like View, App, Scene (and it also covers ViewModifier). It’s an opt-in rule (disabled by default) and supports autocorrection (it can insert private). [1]
Enable it (.swiftlint.yml)
opt_in_rules:
- private_swiftui_stateopt_in_rules is how you enable rules that are disabled by default. [2]
Configure severity
private_swiftui_state:
severity: error # default is warningDefault severity is warning. [1]
Disable (globally or inline)
Global:
disabled_rules:
- private_swiftui_stateInline:
// swiftlint:disable:next private_swiftui_state
`@State` var isPlaying = falseSwiftLint supports // swiftlint:disable … / // swiftlint:enable … (and :next/:this/:previous). [2]
What triggers it (examples): @State var …, @State public var …, @StateObject var …, or @State private(set) var … inside SwiftUI types. [1]
Sources
[1] SwiftLint rule docs: private_swiftui_state (Private SwiftUI State Properties) (realm.github.io)
[2] SwiftLint README: opt_in_rules and swiftlint:disable configuration/comments (github.com)
Citations:
- 1: https://realm.github.io/SwiftLint/private_swiftui_state.html
- 2: https://github.com/realm/SwiftLint
Make @State model private.
The model property should be declared private to encapsulate internal state and follow SwiftUI best practices (private_swiftui_state rule).
Suggested fix
- `@State` var model: Model
+ `@State` private var model: Model📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @State var model: Model | |
| `@State` private var model: Model |
🧰 Tools
🪛 SwiftLint (0.63.2)
[Warning] 4-4: SwiftUI state properties should be private
(private_swiftui_state)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@iosApp/iosApp/Views/Scenes/First/FirstComponent.swift` at line 4, The `@State`
property model on FirstComponent should be made private to encapsulate internal
SwiftUI state; update the declaration of the property named model (of type
Model) inside the FirstComponent view/struct to be private (e.g., change "@State
var model: Model" to a private `@State` declaration) so it follows the
private_swiftui_state rule and prevents external access.
| import SwiftUI | ||
|
|
||
| struct PickerComponent<Model: PickerComponentModelProtocol>: View { | ||
| @State var model: Model |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "PickerComponent.swift" -type fRepository: futuredapp/kmp-futured-template
Length of output: 132
🏁 Script executed:
cat -n iosApp/iosApp/Views/Scenes/Picker/PickerComponent.swift | head -20Repository: futuredapp/kmp-futured-template
Length of output: 911
🌐 Web query:
SwiftUI @State private modifier best practice lint rule
💡 Result:
In Swift, property wrappers like @State are attributes, and the access level (private) is a modifier, so the conventional (and idiomatic) ordering is:
`@State` private var count = 0This is also how most SwiftUI teaching material and examples present it. [2], [3]
Linting it (SwiftLint)
SwiftLint doesn’t have a built-in rule specifically for “@State must be private”, and its modifier_order rule is about modifier consistency (it doesn’t enforce ordering relative to property-wrapper attributes). [4]
A common approach is a custom SwiftLint regex rule that warns when @State var (or other SwiftUI DynamicPropertys) appears without private, e.g. (from a widely-circulated config gist):
custom_rules:
state_private:
name: "Private SwiftUI State"
regex: "\\@State\\s*var"
message: "SwiftUI `@State` variables should always be marked private."
severity: warning…and similar rules for @StateObject, @Environment, etc. [1]
Sources: [1] [2] [3] [4]
🏁 Script executed:
find . -name ".swiftlint.yml" -o -name "swiftlint.yml" -o -name ".swiftlint.yaml" | head -5Repository: futuredapp/kmp-futured-template
Length of output: 98
🏁 Script executed:
cat -n .swiftlint.yml 2>/dev/null | head -100Repository: futuredapp/kmp-futured-template
Length of output: 57
🏁 Script executed:
cat ./iosApp/.swiftlint.ymlRepository: futuredapp/kmp-futured-template
Length of output: 2402
Make SwiftUI state private on the component model.
@State properties should be marked private to follow SwiftUI conventions and encapsulation best practices.
Suggested fix
- `@State` var model: Model
+ `@State` private var model: Model📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @State var model: Model | |
| `@State` private var model: Model |
🧰 Tools
🪛 SwiftLint (0.63.2)
[Warning] 4-4: SwiftUI state properties should be private
(private_swiftui_state)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@iosApp/iosApp/Views/Scenes/Picker/PickerComponent.swift` at line 4, The
`@State` property for the component model (model: Model) is not private; change
its declaration to use private access control so SwiftUI state is encapsulated
(i.e., make the `@State` property private for the Model instance used by the
PickerComponent). Locate the declaration of model (the `@State` var model: Model)
and update it to be private, keeping the same type and semantics so other code
paths using the PickerComponent remain unchanged.
| self.viewModel = viewModel | ||
| } | ||
| struct ProfileComponent<Model: ProfileComponentModelProtocol>: View { | ||
| @State var model: Model |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "ProfileComponent.swift" -type fRepository: futuredapp/kmp-futured-template
Length of output: 134
🏁 Script executed:
cat -n ./iosApp/iosApp/Views/Scenes/Profile/ProfileComponent.swiftRepository: futuredapp/kmp-futured-template
Length of output: 783
🏁 Script executed:
find . -name ".swiftlint.yml" -o -name "swiftlint.yml" -o -name ".swiftlint.yaml" | head -5Repository: futuredapp/kmp-futured-template
Length of output: 98
🏁 Script executed:
cat ./iosApp/.swiftlint.ymlRepository: futuredapp/kmp-futured-template
Length of output: 2402
🏁 Script executed:
rg "private_swiftui_state" ./iosApp/.swiftlint.ymlRepository: futuredapp/kmp-futured-template
Length of output: 57
🏁 Script executed:
web_search
SwiftLint private_swiftui_state ruleRepository: futuredapp/kmp-futured-template
Length of output: 173
🌐 Web query:
SwiftLint private_swiftui_state rule documentation
💡 Result:
SwiftLint’s private_swiftui_state rule documentation is here: “Private SwiftUI State Properties”. It enforces that SwiftUI @State properties should be private (e.g., flags @State var ... / @State public var ... in App, Scene, View, and ViewModifier types). Opt-in, autocorrectable, default severity warning. [1]
To enable it:
opt_in_rules:
- private_swiftui_stateIt’s listed in SwiftLint’s rule directory under “Private SwiftUI State Properties”. [2]
Sources: [1] [2]
Make SwiftUI state private.
@State properties own view state and should not be accessible from outside the view. Line 4 should declare model as private to prevent accidental external mutation.
Suggested fix
- `@State` var model: Model
+ `@State` private var model: Model📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @State var model: Model | |
| `@State` private var model: Model |
🧰 Tools
🪛 SwiftLint (0.63.2)
[Warning] 4-4: SwiftUI state properties should be private
(private_swiftui_state)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@iosApp/iosApp/Views/Scenes/Profile/ProfileComponent.swift` at line 4, The
`@State` property "model" in ProfileComponent (declared as "@State var model:
Model") must be made private to prevent external mutation; change its
declaration to mark it private (e.g., "private `@State` var model: Model") so the
SwiftUI state is only owned by the view and not accessible from other types.
|
|
||
| @Observable | ||
| final class SecondComponentModel: SecondComponentModelProtocol { | ||
| private var viewState: SecondViewState |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if SecondComponent uses any state from SecondComponentModel
rg -n "model\." iosApp/iosApp/Views/Scenes/Second/SecondComponent.swiftRepository: futuredapp/kmp-futured-template
Length of output: 273
🏁 Script executed:
#!/bin/bash
# Read the full SecondComponent.swift to see all usages
cat -n iosApp/iosApp/Views/Scenes/Second/SecondComponent.swiftRepository: futuredapp/kmp-futured-template
Length of output: 882
🏁 Script executed:
#!/bin/bash
# Search for viewState usage anywhere in the Second scene directory
rg -n "viewState" iosApp/iosApp/Views/Scenes/Second/Repository: futuredapp/kmp-futured-template
Length of output: 506
🏁 Script executed:
#!/bin/bash
# Read the full SecondComponentModel.swift file
cat -n iosApp/iosApp/Views/Scenes/Second/SecondComponentModel.swiftRepository: futuredapp/kmp-futured-template
Length of output: 1698
Remove unused viewState subscription
The viewState property is subscribed to and continuously updated, but it remains private and is never used anywhere in the component. This creates unnecessary overhead from the async task running in the background. Either remove the subscription entirely or expose viewState as a public property if the view needs to display this state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@iosApp/iosApp/Views/Scenes/Second/SecondComponentModel.swift` at line 13,
SecondComponentModel has a private viewState property that is subscribed and
updated but never read; remove the unused overhead by either deleting the
private viewState and any async Task/subscription that updates it, or make
viewState public (or `@Published`) and wire it into the view so it is actually
consumed. Locate viewState and the async update logic inside
SecondComponentModel, then either (a) remove the viewState declaration and
remove/stop any Task or Combine subscription that writes to it, or (b) change
its access to public (or `@Published` public) and update the view code to
reference SecondComponentModel.viewState so the subscription is used. Ensure no
dangling tasks/subscriptions remain after the change.
| @Test | ||
| func example() { | ||
| #expect(true) | ||
| } |
There was a problem hiding this comment.
Replace tautological test with behavior verification before merge.
Line 6 (#expect(true)) always passes and cannot catch regressions. Please replace this with at least one deterministic assertion against real app behavior (e.g., state transition, event handling, or navigation contract).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@iosApp/iosAppTests/AppTests.swift` around lines 4 - 7, The test example() in
AppTests.swift currently uses a tautological `#expect`(true); replace it with a
deterministic assertion that verifies real app behavior: call the code path you
want to protect (e.g., instantiate the app or view controller, trigger a state
change or navigation, or call a method) and assert its observable outcome using
XCTest assertions (XCTAssertEqual/XCTAssertTrue/XCTAssertNotNil) — update the
example() test to initialize the relevant class or view controller, perform the
action, and assert the expected state/result so the test can catch regressions.
2 participants
Swift 6.2
Minimum iOS SDK bumped to iOS 17
Naming unified with FuturedKit where possible
ComponentModels mocks
Migrate to @State and @observable
AlertModel from FuturedKit
Summary by CodeRabbit
Release Notes
Requirements
Infrastructure