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

Filter on updates not working #501

Open
sarensw opened this issue Dec 28, 2022 · 3 comments
Open

Filter on updates not working #501

sarensw opened this issue Dec 28, 2022 · 3 comments

Comments

@sarensw
Copy link

sarensw commented Dec 28, 2022

Hi All, given the following class that inherits from StoreSubscriber, I'm trying to subscribe only to a sub-state. But even if another sub-state is updated, then this subscriber is notified about an update.

class PortWindowService: StoreSubscriber {
    typealias StoreSubscriberStateType = ModelState
    
    func start() {
        store.subscribe(self) { $0.select { $0.modelState } }
    }

    deinit {
        store.unsubscribe(self)
    }
    
    func newState(state: ModelState) {
        print(state.ports.count)
        print("ports changed")
    }
}

My app state is:

struct AppState {
    var modelState: ModelState = ModelState()
    var volumeState: VolumeState = VolumeState()
}

struct ModelState {
    var model: StoredModel? = nil
    var ports: [Port] = []
}

struct VolumeState {
    var volumes: [VolumeInfo] = []
}

And my reducers are:

func appReducer(action: Action, state: AppState?) -> AppState {
    return AppState(
        modelState: modelReducer(action: action, state: state?.modelState),
        volumeState: volumeReducer(action: action, state: state?.volumeState))
}
        
func modelReducer(action: Action, state: ModelState?) -> ModelState {
    var state = state ?? ModelState()
    
    switch action {
    case let action as SetModel:
        state.model = action.model
        state.model?.usbPortsLeft.enumerated().forEach { (index, portNumber) in
            state.ports.append(Port(portNumber: portNumber, state: .unknown, area: .left))
        }
        state.model?.usbPortsRight.enumerated().forEach { (index, portNumber) in
            state.ports.append(Port(portNumber: portNumber, state: .unknown, area: .right))
        }
        state.model?.usbPortsBack.enumerated().forEach { (index, portNumber) in
            state.ports.append(Port(portNumber: portNumber, state: .unknown, area: .back))
        }
    default: break
    }
    
    return state
}

func volumeReducer(action: Action, state: VolumeState?) -> VolumeState {
    var state = state ?? VolumeState()
    
    switch action {
    case let action as AddVolume:
        if let volumeInfo = action.volume {
            state.volumes.append(volumeInfo)
        }
    case let action as RemoveVolume:
        if let basePath = action.basePath {
            state.volumes.removeAll(where: { $0.volumePath == basePath })
        }
    default: break
    }
    
    return state
}

The SetModel action is only called twice during startup. So I assume that "ports changed" is only printed twice (see func newState in the PortWindowService class). But even when the actions AddVolume or RemoveVolume are called in a different reducer, in a different sub-state, which is not selected during subscription in the StoreSubscriber initialization, I'll get an event and newState is called.

What am I missing here? How can I make sure that with store.subscribe(self) { $0.select { $0.modelState } } newState is only called when anything in modelState changes, but not in volumeState?

@DivineDominion
Copy link
Contributor

@sarensw This is actually normal behavior (because even when you change a "leaf" in the state, the root value also changes). ReSwift offers to skip identical state updates when the states conform to Equatable. See: http://reswift.github.io/ReSwift/master/getting-started-guide.html#example-of-skipping-identical-state-updates

@sarensw
Copy link
Author

sarensw commented Dec 28, 2022

@DivineDominion , thanks a lot for the quickly reply. Interesting. I tried with .skip(==) and it works. Coming from Typescript + Redux, this seems strange, though.

I took the following selector from some of my TypeScript code.

// only if the reference of .current changes, then update the component
const currentFile = useAppSelector(state => state.files.current)

// only if the length of the .files array changes, then update the component
const count = useAppSelector(state => state.files.files?.length)

So I can be very specific about the updates that I want to be notified about. And I don't have to skip same values (let alone make sure that Equatable is implemented.

No complains here. Just something that I have to learn.

One thing that I also noticed is that I can't just select any arbitrary property that I want. It feels like it has to be struct what I can filter on. But maybe I am also missing something here.

@DivineDominion
Copy link
Contributor

@sarensw Swift is a bit stricter than vanilla JS when it comes to checking for value equality. It may be a good practice to slap "Equatable" onto all your subtypes just so you don't need to worry.

You can select arbitrary substates, but then you need to make them a Swift tuple. These suffer from not being equatable, so you get convenient substate selection but need to (!) add the .skip(==) manually. That's because for tuples between 2 and 6 elements, a == overload exists; but the tuple is not a type that ca conform to Equatable, so ReSwift can't skip it by default. See https://developer.apple.com/documentation/swift/equatable

Having said that:

We could copy the tuple == overload approach and offer substate selector overloads that do this by default, I believe. Requires tests, of course. If you'd be into that, let me know :)

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

No branches or pull requests

2 participants