Skip to content

Commit

Permalink
Guarante that modifiers of a @dependency are available in own modifiers
Browse files Browse the repository at this point in the history
  • Loading branch information
Supereg committed Aug 9, 2024
1 parent 09db45c commit ded33ca
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ import SwiftUI


enum ModifierPlacement: Int, Comparable {
/// No specific order requirement.
case regular
/// Outermost placement (e.g., @Model-based property wrappers).
case outermost

static func < (lhs: ModifierPlacement, rhs: ModifierPlacement) -> Bool {
Expand Down
30 changes: 18 additions & 12 deletions Sources/Spezi/Spezi/Spezi.swift
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,15 @@ public final class Spezi: Sendable {
///
/// Any changes to this property will cause a complete re-render of the SwiftUI view hierarchy. See `SpeziViewModifier`.
@MainActor var viewModifiers: [any ViewModifier] {
_viewModifiers.reduce(into: []) { partialResult, entry in
partialResult.append(entry.value)
}
_viewModifiers
// View modifiers of inner-most modules are added first due to the dependency order.
// However, we want view modifiers of dependencies to be available for inside view modifiers of the parent
// (e.g., ViewModifier should be able to require the @Environment(...) value of the @Dependency).
// This is why we need to reverse the order here.
.reversed()
.reduce(into: []) { partialResult, entry in
partialResult.append(entry.value)
}
}

/// A collection of ``Spezi/Spezi`` `LifecycleHandler`s.
Expand Down Expand Up @@ -230,7 +236,7 @@ public final class Spezi: Sendable {

for module in dependencyManager.initializedModules {
if requestedModules.contains(ModuleReference(module)) {
// the policy only applies to the request modules, all other are always managed and owned by Spezi
// the policy only applies to the requested modules, all other are always managed and owned by Spezi
self.initModule(module, ownership: ownership)
} else {
self.initModule(module, ownership: .spezi)
Expand Down Expand Up @@ -317,20 +323,20 @@ public final class Spezi: Sendable {
module.storeWeakly(into: self)
}

let modifierEntires = module.viewModifierEntires
// this check is important. Change to viewModifiers re-renders the whole SwiftUI view hierarchy. So avoid to do it unnecessarily
if !modifierEntires.isEmpty {
_viewModifiers.merge(modifierEntires) { _, rhs in
rhs
}
}

// If a module is @Observable, we automatically inject it view the `ModelModifier` into the environment.
if let observable = module as? EnvironmentAccessible {
// we can't guarantee weak references for EnvironmentAccessible modules
precondition(ownership != .external, "Modules loaded with self-managed policy cannot conform to `EnvironmentAccessible`.")
_viewModifiers[ModuleReference(module)] = observable.viewModifier
}

let modifierEntires: [(id: UUID, modifier: any ViewModifier)] = module.viewModifierEntires
// this check is important. Change to viewModifiers re-renders the whole SwiftUI view hierarchy. So avoid to do it unnecessarily
if !modifierEntires.isEmpty {
for entry in modifierEntires.reversed() { // reversed, as we re-reverse things in the `viewModifier` getter
_viewModifiers.updateValue(entry.modifier, forKey: entry.id)
}
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion Tests/SpeziTests/DependenciesTests/DependencyTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ final class DependencyTests: XCTestCase { // swiftlint:disable:this type_body_le
}

@MainActor
func testConfigureCallOrder() throws {
func testConfigureCallOrder() throws { // swiftlint:disable:this function_body_length

Check failure on line 665 in Tests/SpeziTests/DependenciesTests/DependencyTests.swift

View workflow job for this annotation

GitHub Actions / SwiftLint / SwiftLint / SwiftLint

Superfluous Disable Command Violation: SwiftLint rule 'function_body_length' did not trigger a violation in the disabled region; remove the disable command (superfluous_disable_command)
class Order: Module, DefaultInitializable {
@MainActor
var order: [Int] = []
Expand Down
11 changes: 4 additions & 7 deletions Tests/UITests/TestApp/ModelTests/ModuleWithModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@ class MyModel2 {
}


private struct MyModifier: ViewModifier {
private struct MyModifier2: ViewModifier {
@Environment(MyModel2.self)
var model
@Environment(ModuleWithModel.self)
var module

nonisolated init() {}

func body(content: Content) -> some View {
content
.environment(\.customKey, model.message == "Hello World" && module.message == "MODEL")
Expand All @@ -43,14 +45,9 @@ class ModuleWithModel: Module, EnvironmentAccessible {
@Model var model = MyModel2(message: "Hello World")

// ensure reordering happens, ViewModifier must be able to access the model from environment
@Modifier fileprivate var modifier: MyModifier
@Modifier fileprivate var modifier = MyModifier2()

let message: String = "MODEL"

@MainActor
init() {
modifier = MyModifier() // @MainActor isolated default values for property wrappers must currently be specified explicitly via isolated init
}
}


Expand Down
40 changes: 39 additions & 1 deletion Tests/UITests/TestApp/ViewModifierTests/ModuleWithModifier.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,55 @@ class MyModel {
}


private struct MyModifier: ViewModifier {
struct MyModifier: ViewModifier {
@Observable
class ModifierState {
@MainActor var appeared: Bool = false

init() {}
}

// We expect that a EnvironmentAccessible dependency is available inside the environment of a modifier
// that is placed in the parent module.
@Environment(ModuleB.self) private var module: ModuleB?

let model: MyModel

@State private var state = ModifierState()

private var alertBinding: Binding<Bool> {
Binding {
module == nil && state.appeared

Check failure on line 41 in Tests/UITests/TestApp/ViewModifierTests/ModuleWithModifier.swift

View workflow job for this annotation

GitHub Actions / Build and Test UI Tests iOS / Test using xcodebuild or run fastlane

main actor-isolated property 'appeared' can not be referenced from a non-isolated autoclosure

Check failure on line 41 in Tests/UITests/TestApp/ViewModifierTests/ModuleWithModifier.swift

View workflow job for this annotation

GitHub Actions / Build and Test UI Tests visionOS / Test using xcodebuild or run fastlane

main actor-isolated property 'appeared' can not be referenced from a non-isolated autoclosure
} set: { _ in
}
}

func body(content: Content) -> some View {
content
.environment(model)
.environment(state)
.alert("Test Failed", isPresented: alertBinding) {
} message: {
Text(verbatim: "ModuleB dependency was not available in the environment of the modifier of the parent!")
}

Check failure on line 54 in Tests/UITests/TestApp/ViewModifierTests/ModuleWithModifier.swift

View workflow job for this annotation

GitHub Actions / SwiftLint / SwiftLint / SwiftLint

Vertical Whitespace before Closing Braces Violation: Don't include vertical whitespace (empty line) before closing braces (vertical_whitespace_closing_braces)
}
}

private class ModuleA: Module, DefaultInitializable {
required init() {}
}

private class ModuleB: Module, EnvironmentAccessible {
@Dependency(ModuleA.self) private var module

init() {}
}


class ModuleWithModifier: Module {
@Dependency(ModuleB.self) private var moduleB = ModuleB()

@Modifier fileprivate var modelModifier: MyModifier

@MainActor
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import XCTSpezi
class MyModelTestCase: TestAppTestCase {
let model: MyModel


init(model: MyModel) {
self.model = model
}
Expand All @@ -30,9 +29,17 @@ class MyModelTestCase: TestAppTestCase {
struct ViewModifierTestView: View {
@Environment(MyModel.self)
var model
@Environment(MyModifier.ModifierState.self)
var state


var body: some View {
TestAppView(testCase: MyModelTestCase(model: model))
.onAppear {
state.appeared = true
}
.onDisappear {
state.appeared = false
}
}
}
3 changes: 2 additions & 1 deletion Tests/UITests/TestAppUITests/ViewModifierTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ final class ViewModifierTests: XCTestCase {
XCTAssertTrue(app.wait(for: .runningForeground, timeout: 2.0))

app.buttons["ViewModifier"].tap()


XCTAssertFalse(app.alerts["Test Failed"].waitForExistence(timeout: 1))
XCTAssert(app.staticTexts["Passed"].waitForExistence(timeout: 1))
}
}

0 comments on commit ded33ca

Please sign in to comment.