Skip to content

feat: add onChange overload that ignores old and new values #3681

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MojtabaHs
Copy link

In my experience, most of the time, the old and new values in onChange aren’t needed.
Following SwiftUI’s design for onChange, I’ve added a simplified overload that ignores both values.

Internally, this new overload calls the original function to maintain a single source of truth.

@stephencelis
Copy link
Member

Hi @MojtabaHs, thanks for taking the time to contribute! I think we're down to accept this additional API, but could you first provide a motivating example or two where neither the old nor new value are needed?

@MojtabaHs
Copy link
Author

Hi @stephencelis, this usually needed when the reducer logic is independent of the changing value or when the action is self-contained. Here are a couple of examples to show what I mean:

  • Case 1: Triggering Side Effects, Not State Changes
.onChange(of: \.level) { _, _ in
    Reduce { state, action in
        .run { send in
            try await leaderboardClient.triggerUpdate()
        }
    }
}

In this case, gameCenter (or leaderboardClient) already knows the current score, so we’re just using this as a trigger. We don’t need the new level value itself.


  • Case 2: Derived Logic from Multiple Properties
BindingReducer()
    .onChange(of: \.hours) { _, _ in normalizeTheDuration() }
    .onChange(of: \.minutes) { _, _ in normalizeTheDuration() }

Here, normalizeTheDuration() uses several properties together. We only care that something changed, not what it changed to.


Sure, there are other ways to do this (like a computed property or combining the triggers), but I think this API fits naturally with SwiftUI’s style, and clean.

It’s a helpful way to handle “run logic on change” scenarios without overcomplicating the reducer. Definitely feels like something that belongs in the core.

@stephencelis
Copy link
Member

And normalizeTheDuration() returns a reducer? Can you expand that part of the code so we can better evaluate?

I do think this API is mostly harmless to add, but one concern is that this overload may affect reducer builders using the existing onChange operator. Overloads can strain the compiler, especially in result builder contexts, and so merging this does have the potential to break existing code by creating an expression that takes too long to type-check. One option is to merge, release, and revert if there's a regression. It'd be unfortunate to have to do a breaking change via the revert, though, so we're going to think on things just a little longer.

@MojtabaHs
Copy link
Author

The extracted logic would be something like:

func normalizeTheDuration() -> Reduce<State, Action> {
    Reduce { state, action in
        let needsNormalization = state.hours * 60 + state.minutes < minimumMinutes
        if needsNormalization {
            state.minutes = minimumMinutes % 60
            state.hours = Int(Double(minimumMinutes) / 60)
        }
        return .none
    }
}

or:

.onChange(of: \.hours) { _, _ in  Reduce { state, action in normalizeTheDuration() } }
.onChange(of: \.minutes) { _, _ in  Reduce { state, action in normalizeTheDuration() } }

We can implement this PR with a simple extension since it literally extends the original behavior:

extension Reducer {
    @inlinable
    public func onChange<V: Equatable, R: Reducer>(
        of toValue: @escaping (State) -> V,
        @ReducerBuilder<State, Action> _ reducer: @escaping () -> R
    ) -> _OnChangeReducer<Self, V, R> {
        onChange(of: toValue) { _, _ in reducer() }
    }
}

I didn't see any issues for auto complete or a warning on -warn-long-expression-type-checking=100. Xcode happily shows the auto complete suggestion as:
Screenshot 2025-05-14 at 1 43 00 AM

The environment is a project with multi-module (about 10 SPM targets) architecture and about 30 TCA features.
Please let me know if there is anything I can do to have a reliable benchmark or something.

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.

2 participants