Fix the issue where AtomEffectBuilder fails to demangle witness for the resulting type of multiple blocks #193
Conversation
…he return type of the builder with multiple blocks in iOS 16
There was a problem hiding this comment.
Pull Request Overview
This PR addresses a runtime crash on iOS 16 by replacing the variadic opaque return type in buildBlock(_:) with a concrete BlockEffect and adds a zero-argument overload returning EmptyEffect. It also cleans up legacy conditional types by removing BlockBCEffect and the availability branch.
- Added
buildBlock()overload returningEmptyEffectfor empty cases - Converted variadic
buildBlock<each Effect>to return a concreteBlockEffect - Removed legacy
BlockBCEffectand iOS 17 availability checks, movedBlockEffectinto public API
Comments suppressed due to low confidence (1)
Sources/Atoms/Effect/AtomEffectBuilder.swift:22
- No tests cover the new zero-argument
buildBlock()path. Consider adding a unit test to verify thatbuildBlock()returns anEmptyEffectand that it behaves as a no-op effect.
public static func buildBlock() -> some AtomEffect {
| @@ -54,6 +56,46 @@ public enum AtomEffectBuilder { | |||
| } | |||
|
|
|||
| public extension AtomEffectBuilder { | |||
| // Use type pack once it is available in iOS 17 or newer. | |||
| // struct BlockEffect<each Effect: AtomEffect>: AtomEffect | |||
| struct BlockEffect: AtomEffect { | |||
There was a problem hiding this comment.
Consider annotating BlockEffect (and its initializer and methods) with @MainActor to enforce actor isolation and prevent potential threading issues, aligning with the @MainActor context of AtomEffectBuilder.
Copilot uses AI. Check for mistakes.
| _initializing = { context in | ||
| repeat (each effect).initializing(context: context) | ||
| } | ||
| _initialized = { context in | ||
| repeat (each effect).initialized(context: context) | ||
| } | ||
| _updated = { context in | ||
| repeat (each effect).updated(context: context) | ||
| } | ||
| _released = { context in | ||
| repeat (each effect).released(context: context) | ||
| } |
There was a problem hiding this comment.
[nitpick] The initializer and the related closure properties (_initializing, _initialized, etc.) are repetitive. Consider abstracting the repeated logic—e.g., iterating over a collection of effects in a single closure—to reduce boilerplate.
| _initializing = { context in | |
| repeat (each effect).initializing(context: context) | |
| } | |
| _initialized = { context in | |
| repeat (each effect).initialized(context: context) | |
| } | |
| _updated = { context in | |
| repeat (each effect).updated(context: context) | |
| } | |
| _released = { context in | |
| repeat (each effect).released(context: context) | |
| } | |
| _initializing = createEffectHandler(effect: repeat each effect, method: { $0.initializing(context:) }) | |
| _initialized = createEffectHandler(effect: repeat each effect, method: { $0.initialized(context:) }) | |
| _updated = createEffectHandler(effect: repeat each effect, method: { $0.updated(context:) }) | |
| _released = createEffectHandler(effect: repeat each effect, method: { $0.released(context:) }) | |
| } | |
| private func createEffectHandler<each Effect: AtomEffect>( | |
| effect: repeat each Effect, | |
| method: @escaping (Effect) -> (Context) -> Void | |
| ) -> @MainActor (Context) -> Void { | |
| return { context in | |
| repeat method(each effect)(context) | |
| } |
Copilot uses AI. Check for mistakes.
Pull Request Type