Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Obsolete Coordinator #136

Merged
merged 3 commits into from
May 22, 2024
Merged

Obsolete Coordinator #136

merged 3 commits into from
May 22, 2024

Conversation

ra1028
Copy link
Owner

@ra1028 ra1028 commented May 15, 2024

Description

The Coordinator API had greatly increased the complexity of the internal code compared to its low utilization, so we should be discontinuing it in this version.
AtomEffect, which is introduced in this version, covers the main uses of Coordinator, and it should be possible to make other uses of it work the same way by using an object managed by separate atom.

@ra1028 ra1028 force-pushed the feat/obsolete-coordinator branch from 39e5a6d to 3300526 Compare May 15, 2024 15:23
Copy link
Collaborator

@rasberik rasberik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gimme-gimme

@ra1028 ra1028 marked this pull request as ready for review May 22, 2024 06:13
@ra1028 ra1028 merged commit 330123c into main May 22, 2024
6 checks passed
@ra1028 ra1028 deleted the feat/obsolete-coordinator branch May 22, 2024 06:28
@strangeliu
Copy link

I had to say i just miss the Coordinator API. : )
IMP, The SwiftUI like API

func makeCoordinator()
func make(context: Context)
func update(context: Context)
static func dismantle(context: Context)

is really a good and clean design.

While working with the new AtomEffect API, I noticed a lifecycle-related pattern that might benefit from discussion regarding API design ergonomics.

The Pain Point:
For atoms requiring setup/teardown logic (e.g., BatteryStatusAtom), I'm forced to perform side effects in defaultValue (enabling battery monitoring) which violates the purity expectation of this function. Ideally, initialization logic should have a dedicated hook (similar to Coordinator's make in SwiftUI).

Example Implementation:

struct BatteryStatusAtom: StateAtom, Hashable {
    func defaultValue(context: Context) -> Float {
        // ❌ Side effects don't belong in a pure function
        UIDevice.current.isBatteryMonitoringEnabled = true
        return UIDevice.current.batteryLevel
    }
    
    func effect(context: CurrentContext) -> BatteryObserveEffect {
        BatteryObserveEffect()
    }
    
    class BatteryObserveEffect: AtomEffect {
        func released(context: Context) {
            // ✅ Proper cleanup location
            UIDevice.current.isBatteryMonitoringEnabled = false
        }
    }
}

Proposed Solution:
Would it be feasible to introduce lifecycle methods like willCreate/didCreate in AtomEffect,

This could make side effect management more intentional while preserving defaultValue purity. I'm curious to hear your perspective!

@ra1028
Copy link
Owner Author

ra1028 commented Mar 1, 2025

@strangeliu

I believe that the coordinator's make was also not a good place to make side effects.
Anyways, adding one more effect function to the AtomEffect that's called before the value creation makes sense to me.
Can you please issue a feature request so I don't miss it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants