Skip to content

Add FXIOS-14446 [Performance] Make all states equatable #31278

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.

Sign up for GitHub

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
Cramsden wants to merge 2 commits into
base: main
Choose a base branch
from
+9 −5
Open

Add FXIOS-14446 [Performance] Make all states equatable #31278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1973e40
Make all states equatable
Cramsden Dec 12, 2025
0ede11b
add diff check to Subscription newValues
Cramsden Dec 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion BrowserKit/Sources/Redux/State.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import Foundation
/// Defines the entire application state including the UI state and any model state that you used in the app.
/// This state is stored inside of the `Store`, then you have views and other subscribers that will get notified
/// every single time that state updates into the entire app.
public protocol StateType: Sendable {
public protocol StateType: Sendable, Equatable {
/// Returns a default State by clearing any transient data from previous one.
///
/// All the state properties that have a default value into the initializer should be restore to default.
Expand Down
8 changes: 6 additions & 2 deletions BrowserKit/Sources/Redux/Subscription.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import Foundation

@MainActor
final class SubscriptionWrapper<State>: @MainActor Hashable {
final class SubscriptionWrapper<State: Equatable>: @MainActor Hashable {
private let originalSubscription: Subscription<State>
weak var subscriber: AnyStoreSubscriber?
private let objectIdentifier: ObjectIdentifier
Expand Down Expand Up @@ -43,7 +43,7 @@ final class SubscriptionWrapper<State>: @MainActor Hashable {
}

@MainActor
public final class Subscription<State> {
public final class Subscription<State: Equatable> {
public var observer: (@MainActor (State?, State) -> Void)?

init() {}
Expand All @@ -55,6 +55,10 @@ public final class Subscription<State> {
}

func newValues(oldState: State?, newState: State) {
// State was updated but observers are not notified if the state is the same
guard newState != oldState else {
return
}
Comment on lines +58 to +61
Copy link
Contributor Author

@Cramsden Cramsden Dec 12, 2025

Choose a reason for hiding this comment

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

@mattreaganmozilla I was thinking it could be good to have a log here at least in the beginning for debugging purposes but I am not sure if it is overkill or going to be weird to pass the logger to this level.

Copy link
Collaborator

@mattreaganmozilla mattreaganmozilla Dec 12, 2025

Choose a reason for hiding this comment

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

Honestly I think that would create too much spam in the console. 🤔 I had worked on a logger throttler for de-dup'ing messages a while back but never merged it (pretty sure this is the PR: #22547).

We could consider rolling that back in perhaps, it would probably help in scenarios like this.

self.observer?(oldState, newState)
}

Expand Down
4 changes: 2 additions & 2 deletions firefox-ios/Client/Redux/GlobalState/ActiveScreenState.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import Foundation
import Redux
import Common

enum AppScreenState: Sendable {
enum AppScreenState: Sendable, Equatable {
case browserViewController(BrowserViewControllerState)
case homepage(HomepageState)
case mainMenu(MainMenuState)
Expand Down Expand Up @@ -100,7 +100,7 @@ enum AppScreenState: Sendable {
}
}

struct ActiveScreensState: Sendable {
struct ActiveScreensState: Sendable, Equatable {
let screens: [AppScreenState]

init() {
Expand Down