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

Best practice for network/asynchronous call #214

Open
Dimillian opened this issue Mar 7, 2017 · 25 comments

Comments

Projects
None yet
@Dimillian
Copy link

commented Mar 7, 2017

I'm debating if I'm doing the right thing by putting my network call in what I call NetworkAction (well async action).
If I understand correctly, in a true redux style implementation, I should put my network call in a middleware, right?

For the moment I'm doing something like this

struct FetchUser: Action {
    init(user: String) {
        GETRequest(path: Endpoint.Users.user(id: user).path())
            .run { (response: APIResponse<UserJSON>) in
            store.dispatch(SetUser(user: response.object))
        }
    }
}

struct SetUser: Action {
    let user: UserJSON?
}

So basically my FetchUser action is not caught by any reducer, it's just something I call to actually Fetch the user, and the user is reduced once set with the SetUser action.

I'm I missing something important or an implementation like this is ok?

Thanks!

@danielmartinprieto

This comment has been minimized.

Copy link

commented Mar 7, 2017

Hi!

  • You don't need a middleware to dispatch networks calls in an idiomatic Redux implementation. Some people use a middleware as a help to dispatch other actions automatically (loading, success, error), or avoid making the same call again if it's already in progress, etc.
  • If the action is not something needed by any reducer in your app, it probably means that you don't need that action, I mean, that object/function doesn't need to conform the Action protocol.
  • I suggest you just create a function and call it when you want to do the request, and dispatch the real action when you have the user back. Something like:
// Put this function where it makes sense in your code
func loadUser(user: String) {
    // Here you could dispatch another action, to save in the state of your app that you're making the request, e.g.:
    // store.dispatch(UserLoading())
    GETRequest(path: Endpoint.Users.user(id: user).path())
        .run { (response: APIResponse<UserJSON>) in
            store.dispatch(SetUser(user: response.object))
                // You also could dispatch another action to handle
                // the case where the network call doesn't succeed, e.g:
                // store.dispatch(LoadUserFailed(error: response.error)
         }
}
@johnnysparks

This comment has been minimized.

Copy link

commented Mar 8, 2017

@Dimillian Your approach is identical to what I've been using, mostly because FetchUser usually triggers a UI update. While there are probably good reasons not to, I've been putting my cancellable requests in the app state:

struct FetchUser: Action {
    let task: GETRequest
    let user: String
    init(user: String) {
        self.user = user
        task = GETRequest(path: Endpoint.Users.user(id: user).path())
            .run { (response: APIResponse<UserJSON>) in
            store.dispatch(SetUser(user: response.object))
        }
    }
}

struct SetUser: Action {
    let user: UserJSON?
}

struct UserState: StateType {
   var tasks: [String:GETRequest] = [:]
}

func handleUserAction(action: Action, state: UserState? ) -> UserState {
  let state = state ?? UserState()
  switch action {
    case let action as FetchUser:
       if let task = state.tasks[action.user] {
          task.cancel()
          state.tasks[action.user] = nil
       }
       state.tasks[action.user] = action.task
     case let action as SetUser:
       state.tasks[action.user.userId] = nil
     default:
       break
  }
}

The consequence is that sometimes I'm using reference types where it would be better to use structs, and cancelling tasks kinda feels like a side-effect. Maybe it should be handled in middleware? Not sure! I just know that I don't want to be managing request state in my view controllers. 😄 I'd be interested to hear how others are handling this case.

@Dimillian

This comment has been minimized.

Copy link
Author

commented Mar 9, 2017

@johnnysparks Yeah, in my case I'm just dispatching action and my views/controller don't care about the loading state etc.... juste subscribed to the specific state and wait for the data to be available or on error mode.
Why are you storing the task in your state?
My UsersState look like that

struct UsersState: StateType, Equatable {
    var authState: AuthState
    var users: [ObjectId: User] = [:]
    var currentUser: String?
}

func == (lhs: UsersState, rhs: UsersState) -> Bool {
    return lhs.users == rhs.users &&
        lhs.authState == rhs.authState &&
}

and my reducer transformer the UserJSON into a final User

func usersReducer(state: UsersState?, action: Action) -> UsersState {
    var state = state ?? initialUsersState()
    switch action {
    case let action as SetUser:
        if let user = action.user {
            state.users[user.id] = User(json: user)
        }
    default:
        break
    }

    return state
}

In the end I can have a working Login view controller as simple as that

class LoginViewController: UIViewController {

    @IBOutlet var usernameField: UITextField!
    @IBOutlet var passwordField: UITextField!

    override func viewDidLoad() {
        super.viewDidLoad()

        store.subscribe(self) {
            $0.select{ $0.usersState }.skipRepeats()
        }
    }

    @IBAction func onLoginButton(_ sender: Any) {
        store.dispatch(AuthenticatePassword(username: usernameField.text!, password: passwordField.text!))
    }

    @IBAction func onTwitterButton(_ sender: Any) {
        store.dispatch(AuthenticateTwitter())
    }

    @IBAction func onFacebookButton(_ sender: Any) {
        store.dispatch(AuthenticateFacebook(from: self))
    }
}

// MARK: - State management
extension LoginViewController: StoreSubscriber {
    func newState(state: UsersState) {
        if let error = state.authState.error {
            presentError(error: error.type, viewController: self, completion: nil)
        }
        if let _ = state.getCurrentUser {
            self.dismiss(animated: true, completion: nil)
        }
    }
}
@johnnysparks

This comment has been minimized.

Copy link

commented Mar 9, 2017

Thanks @Dimillian! 😳 skipRepeats() is new to me, and will solve the issue I have with, say, multiple "pull to refresh" attempts in quick succession. The reason I sometimes have tasks in my state is for cancelling requests that are no longer needed. If you had a signup form instead of a login form, and you wanted to check username availability on each keystroke:

class SignupViewController: UIViewController, UITextFieldDelegate {

    @IBOutlet var usernameField: UITextField!
    @IBOutlet var usernameValidationLabel: UILabel!
    @IBOutlet var passwordField: UITextField!

    // MARK - UITextFieldDelegate
    // ⬇️ This could fire many times before completing a single request 
    func textViewDidChange(_ textView: UITextView) {
        store.dispatch(CheckUsername(username: textView.text))
    }

    override func viewDidLoad() {
        super.viewDidLoad()
        usernameField.deleagte = self
        store.subscribe(self) {
            $0.select{ $0.usersState }.skipRepeats()
        }
    }

    @IBAction func onLoginButton(_ sender: Any) {
        store.dispatch(AuthenticatePassword(username: usernameField.text!, password: passwordField.text!))
    }
}

// MARK: - State management
extension SignupViewController: StoreSubscriber {
    func newState(state: UsersState) {
        if let error = state.authState.error {
            presentError(error: error.type, viewController: self, completion: nil)
        }
        if let _ = state.getCurrentUser {
            self.dismiss(animated: true, completion: nil)
        }

        // ⬇️ Tell the user that username is available, or if they need to pick another
        if let isValid = state.isUsernameValid, let username = state.username {
            usernameValidationLabel.text = isValid ? "Available!" : "\(username) is taken"
        }
    }
}

If I were building this today, I might have an action called CheckUsername that creates a request task to be cancelled if a new CheckUsername action is fired before the request completes. My new actions might be:

struct CheckUsername: Action {
  let username: String
  let request: GETRequest
  init(username: String) {
    self.username = username
    request = GETRequest(path: "username/\(username)", completionHandler: { isValid, error in
      // ignoring error and response parsing for example
      store.dispatch(ValidatedUsername(username: username, isValid: isValid))
    })
  }
}

struct ValidatedUsername: Action {
  let username: String
  let isValid: Bool
}

And I would store the usernameRequest is in the UserState so that I can cancel it in the reducer.

struct UsersState: StateType, Equatable {
    var authState: AuthState
    var users: [ObjectId: User] = [:]
    var currentUser: String?

    // Username validation
    var username: String?
    var isUsernameValid: Bool?
    var usernameRequest: GETRequest?
    // ⬆️ the cancelable request task (in my case) sits in the state
}

The only time I need to reference that usernameRequest again is in the case of a second CheckUsername action firing with different text as the user types

store.dispatch(CheckUsername(username: "johnnyspa"))
store.dispatch(CheckUsername(username: "johnnyspar"))
store.dispatch(CheckUsername(username: "johnnyspark"))

Then, in the reducer, if a usernameRequest is already there, I cancel it, and replace it with the new CheckUsername request task.

After the requests completes, ValidatedUsername is fired, and I can clear out the completed request task.

func usersReducer(state: UsersState?, action: Action) -> UsersState {
    var state = state ?? initialUsersState()
    switch action {
    case let action as CheckUsername:

      // Confusion is here ⬇️ if I want to cancel an incomplete network request, is this the right way to handle it?
      if state.usernameRequest != nil {
        state.usernameRequest.cancel()
        state.usernameRequest = action.request
      }
      state.username = action.username

    case let action as ValidatedUsername:
      state.isUsernameValid = action.isValid
      state.username = action.username
      state.usernameRequest = nil

    default:
        break
    }

    return state
}

It comes up a handful of times in each app I work on, situations like:

  • giving the user an option to cancel an image upload
  • suggested search results
  • username checking

I'm a bit stumped on the best way to handle it, any suggestions would be appreciated.

Sorry if this derails the original question, and thanks for taking the time to share your code and thoughts!

@Dimillian

This comment has been minimized.

Copy link
Author

commented Mar 10, 2017

Ah yeah SkipRepeats is currently not merged with master: #203
All my state implement Equatable, so using the skipRepeat branch, I can subscribe to substate and only be notified when this specific state/value change, and not when anything change anywhere. It's a big boost performance wise if your state change often. (Using it into a game, with a timer updating some UIState multiple state per second).

For your CheckUsername username flow, I wouldn't bother canceling your request. But I would implement it in a more Redux style flow. Here is how my search work, you'll understand:

Here is my state

struct SearchState: StateType, Equatable {
    var books: [Query: [ObjectId]] = [:]
}

My async actions

struct FetchSearchBooks: Action {

    let query: String
    let offset: Int
    let limit: Int

    init(query: String, offset: Int, limit: Int) {
        self.query = query
        self.offset = offset
        self.limit = limit

        let params = ["q": query as AnyObject,
                      "offset": offset as AnyObject,
                      "limit": limit as AnyObject] as [String : AnyObject]
        GETRequest(path: Endpoint.Search.books.path(),
                   params: params).run { (response: APIResponse<BooksHitsJSON>) in
                    store.dispatch(SetSearchBooks(query: query, response: response))
        }
    }
}

My actions

struct SetSearchBooks: Action {
    let query: String
    let response: APIResponse<BooksHitsJSON>
}

and my reducer

func searchReducer(state: SearchState?, action: Action) -> SearchState {
    var state = state ?? SearchState()

    switch action {
    case let action as SetSearchBooks:
        if let json = action.response.object {
            let ids = json.hits.map {$0.id!}
            state.books[action.query] = ids
        }

    default:
        break
    }
    return state
}

My final books object are reduced with my BooksReducer in my booksState.

All that just to say that I think the best way is to store the parsed response associated with the query. So you can always be in sync with your controller, at each keystroke, check your state if you already have a response, or fire a request for it. You should also debounce it, only fire a request after 0.5s of no key pressed. It's more efficient and less resources intensive.

I don't know specifically your constraints, but I never really wanted or cancelled requests, the extra layer to be able to handle it is not worth it I think. Unless you're in a very constrained environment and every bits of data matter.

@DivineDominion

This comment has been minimized.

Copy link
Contributor

commented May 2, 2017

Update: Now read #240, and it seems you know this stuff already :)


This makes me wonder: why do you dispatch an action if all it does is (1) contain information nobody else needs, and (2) perform an async request that dispatches a more interesting action (from a store mutating point of view).

The actions that perform side-effects upon initialization (that'd be an anti-pattern on its own as far as I am concerned) look like they really want to be action creators.

// ReSwift 3
open func dispatch(_ asyncActionCreatorProvider: @escaping (State, ReSwift.Store<State>, @escaping (((State, ReSwift.Store<State>) -> Action?) -> Swift.Void)) -> Swift.Void)

// ReSwift 4
open func dispatch(_ asyncActionCreatorProvider: @escaping AsyncActionCreator)

(The signature of v3 is brutal.)

Result (not tested to compile):

struct FetchSearchBooks: Action {

    let query: String
    let offset: Int
    let limit: Int

    init(query: String, offset: Int, limit: Int) {
        self.query = query
        self.offset = offset
        self.limit = limit
    }

    func actionCreator(callback: @escaping (ActionCreator) -> Action?) {
        let params = ["q": query as AnyObject,
                      "offset": offset as AnyObject,
                      "limit": limit as AnyObject] as [String : AnyObject]
        GETRequest(path: Endpoint.Search.books.path(),
                   params: params).run { (response: APIResponse<BooksHitsJSON>) in

                    callback { _, _, in
                        return SetSearchBooks(query: query, response: response)
                    }
        }
    }
}

// ...

let search = FetchSearchBooks(...)
let asyncActionCreator: Store<YourMainAppState>.AsyncActionCreator = { _, _, callback in
    search.actionCreator(callback: callback)
}
store.dispatch(asyncActionCreator)
@mpsnp

This comment has been minimized.

Copy link

commented May 2, 2017

@DivineDominion Maybe it's better to keep actions pure (without any action creators), and wrap action creators in separate classes (now i call them Actors 😄, for example TodosActor which group action creators logically). Also, encapsulate requests themselves in "services", this gives us opportunity to mock them easily in tests.
Let me show you:

class TodosActor {    
    let todosService: TodosService
    // We have possibility to inject here stub service in tests
    private init(todosService: TodosService) {
        self.todosService = todosService
    }
    
    // Action creator itself
    func getTodos() -> ActionCreator<Promise<Void>> {
        return { store in
            return self.todosService.getTodos().then { todos in
                store.dispatch(SetTodosAction(todos: todos))
            }
        }
    }

    func add(todo: Todo) -> ActionCreator<Promise<Void>> {
        return { store in
            // dispatching action in order to achieve immediate feedback
            store.dispatch(AddTodoAction(todo: todo))
            
            return self.todosService.add(todo: todo).recover { (error) -> Void in
                // In case of failure, delete it or whatever you want (better to report error of course)
                store.dispatch(DeleteTodoAction(id: todo.id))
                throw error
            }
        }
    }
}

And then you can use it in VCs for example:

class TodosListViewController: UITableViewController {
    // Injecting them somehow, for example using any DI container or at least assigning singletons
    let store: TodosStore!
    let actor: TodosActor!
    
    override func viewDidLoad() {
        super.viewDidLoad()
        store.dispatch(actor.getTodos())
    }
    ....
}

That's it. What do you think?

@DivineDominion

This comment has been minimized.

Copy link
Contributor

commented May 20, 2017

I was pondering this for a while. Extracting the TodosService is a good idea, sure. But these actors, being only action creator factories ...?

Now I have quite a few "pending request" states in my app. Like FileRemoval, which can be enqueued in a PendingFileChange substate. This queue is processed by a store subscriber that performs actual deletion of files on disk.

Stuff like this (enqueueing/dequeueing) gets boring pretty fast. And a lot of the stuff is simple once you separate the store subscriber from the service that performs the action. Which, in turn, works similarly to your Actors -- only you have a central factory for actions targeting a sub-domain (here, To-Dos) while I have subscribers which delegate to services and dispatch process and completion (or failure) actions.

I will look for opportunities to try your Actors/Factory approach in my app and report back. It certainly looks like a lot less boilerplate code -- and a lot less actions I need to write (enqueue + dequeue, or request + completion pairs, plus actual effects).


On a totally different note, this part of your code looks interesting:

    func add(todo: Todo) -> ActionCreator<Promise<Void>> {
        return { store in
            // dispatching action in order to achieve immediate feedback
            store.dispatch(AddTodoAction(todo: todo))
            
            return self.todosService.add(todo: todo).recover { (error) -> Void in
                // In case of failure, delete it or whatever you want (better to report error of course)
                store.dispatch(DeleteTodoAction(id: todo.id))
                throw error
            }
        }
    }

You immediately dispatch AddTodoAction and only upon failure delete it from the state again. That makes sense to have an "optimistic" UI that's super responsive. But couldn't you leave that inside the UI instead of changing the app state? Because until the service finished with success, is this part of the state really the truth, or is it just an intermediate guess?

As explained above, in the line of thought I adhered to in my latest app so far, I would tend to let the TodoService dispatch the error and success (in this case: AddTodoAction) itself, making it autonomous. Your approach is similar, except there's a callback. And the service in turn doesn't have to know the store. (I'd inject it in add(todo:,store:).)

So this is interesting for sure and I'd love to hear more about this!

@wpK

This comment has been minimized.

Copy link

commented May 24, 2017

Since the discussion has dived into optimistic updates, this might be something interesting to look into: https://github.com/mattkrick/redux-optimistic-ui

@Dimillian

This comment has been minimized.

Copy link
Author

commented May 31, 2017

@DivineDominion I did that mostly because the code is kept super simple. I have now pure Action for my "set" actions, and ActionsRequests.
Sure my ActionsRequests are not necessarily reduced and so don't necessarily mutate the state, but they contain useful information, and can be reduced if useful. Which will be very handy for optimistic update. So I can keep my UI code very simple, like this login screen:

class LoginViewController: UIViewController {

    @IBOutlet var usernameField: UITextField!
    @IBOutlet var passwordField: UITextField!

    override func viewDidLoad() {
        super.viewDidLoad()

        store.subscribe(self) {
            $0.select{ $0.usersState }.skipRepeats()
        }
    }

    @IBAction func onLoginButton(_ sender: Any) {
        store.dispatch(AuthenticatePassword(username: usernameField.text!, password: passwordField.text!))
    }

    @IBAction func onTwitterButton(_ sender: Any) {
        store.dispatch(AuthenticateTwitter())
    }

    @IBAction func onFacebookButton(_ sender: Any) {
        store.dispatch(AuthenticateFacebook(from: self))
    }
}

// MARK: - State management
extension LoginViewController: StoreSubscriber {
    func newState(state: UsersState) {
        if let error = state.authState.error {
            presentError(error: error.type, viewController: self, completion: nil)
        }
        if state.getCurrentUser != nil {
            self.dismiss(animated: true, completion: nil)
        }
    }
}

and an auth action which would look like that

// MARK: Actions
struct AuthenticatePassword: Action {
    let username: String
    let password: String

    init(username: String, password: String) {
        self.username = username
        self.password = password
        authenticate()
    }

    func authenticate() {
        AuthRequest(username: self.username, password: self.password).login { (response) in
            if let token = response.token, response.success {
                store.dispatch(SetAuthState(authState: AuthState(loggedIn: true, token: token, error: nil)))
                store.dispatch(FetchCurrentUser())
            } else {
                store.dispatch(SetAuthState(authState: AuthState(loggedIn: false,
                                                                 token: nil,
                                                                 error:
                    ErrorType.invalidAuthPassword.error(userInfo: nil))))
            }
        }
    }
}

To me it looks clean, but maybe I'm doing something very wrong. I still don't see the benefit of ActionCreators :( over this. That weird callback call seems to be too much.

@DivineDominion

This comment has been minimized.

Copy link
Contributor

commented Jun 2, 2017

@wpK I'm not a proficient ES6 coder or reader, but do I get this right?

  1. The library wraps the default app state in a state object with 2 parts, the usual or real "backend" state on one side, and a "frontend-modified" version on the other.
  2. When you dispatch actions optimistically, you provide the estimated state as well.
  3. You consume the optimistic "frontend" version right away in the UI.
  4. You let the library process the action on the backend state; if the frontend state matches to the backend, the UI won't get an update (?); if it differs, the optimistic version will be discarded in favor of the truer truth.
@DivineDominion

This comment has been minimized.

Copy link
Contributor

commented Jun 19, 2017

@Dimillian hmm, but now store is a global variable for the authentication actions, isn't it?

@timojaask

This comment has been minimized.

Copy link

commented Jun 27, 2017

In all of these examples, except for the one by @mpsnp, the functions that dispatch async operations are strongly coupled with networking service. How do you unit test that? I was trying to figure out if there would be a way to use protocol for networking service, so it could be replaced in unit tests, but I couldn't find any good pattern yet. Any ideas?

@timojaask

This comment has been minimized.

Copy link

commented Jun 28, 2017

Finally, I've managed to make all of my ReSwift related logic fully testable.

Now my actions are simple structs, reducers are pure functions and I can test my async code by passing it a test implementation of a service. I'm still working on naming things better, but here's a working example:

// Somewhere in your codebase, wherever you instantiate your store
let store = Store<AppState>(reducer: appReducer, state: nil)
let asyncRequestHandler = AsyncRequestHandler(dataService: TestDataService(), store: store)
store.subscribe(asyncRequestHandler!)
store.dispatch(SetFetchDataState(.request))


// Data type indicating the state of asynchronous action
enum FetchDataState {
    case none
    case request
    case success(data: String)
    case error(error: Error)
}

// Action
struct SetFetchDataState: Action {
    let state: FetchDataState
    init(_ state: FetchDataState) { self.state = state }
}

// Reducer
func appReducer (action: Action, state: AppState?) -> AppState {
    var state = state ?? AppState()
    switch action {
    case let action as SetFetchDataState:
        state.fetchDataState = action.state
        if case let FetchDataState.success(data) = action.state {
            state.remoteData = data
        }
    default:
        break
    }
    return state
}

// State
struct AppState: StateType {
    var remoteData = ""
    var fetchDataState = FetchDataState.none
}

// State change handler that fires up async operations
class AsyncRequestHandler: StoreSubscriber {
    let dataService: DataService
    let store: DispatchingStoreType

    init(dataService: DataService, store: DispatchingStoreType) {
        self.dataService = dataService
        self.store = store
        store.subscribe(self)
    }

    func newState(state: AppState) {
        if case FetchDataState.request = state.fetchDataState {
            dataService.fetchData()
                .then { self.store.dispatch(SetFetchDataState(.success(data: $0))) }
                .catch { self.store.dispatch(SetFetchDataState(.error(error: $0))) }
        }
    }
}

// Service protocol, so you can use different implementations for your tests and production app
protocol DataService {
    func fetchData() -> Promise<String>
}

struct TestDataService: DataService {
    func fetchData() -> Promise<String> {
        return Promise<String> { fulfill, reject in
            DispatchQueue.main.asyncAfter(deadline: DispatchTime.now() + 2) {
                fulfill("Hello from test data service")
            }
        }
    }
}

EDIT: improved naming of things

@timojaask

This comment has been minimized.

Copy link

commented Jun 28, 2017

I have created an example project that demonstrates this pattern. The project includes unit tests. The reducers and actions are free of side effects and the class that handles asynchronous operations is fully tested.

https://github.com/timojaask/ReSwiftTestableAsyncPattern

@DivineDominion

This comment has been minimized.

Copy link
Contributor

commented Jul 7, 2017

Great, thanks for putting all this together, @timojaask!

@Ben-G @mjarvis: With regard to #268, couldn't this be the base of a good example that the guide can explain step-by-step instead of the rather boring counter stuff? If you agree but if @timojaask doesn't want to put more time into the docs, I'd happily volunteer.

@timojaask

This comment has been minimized.

Copy link

commented Jul 7, 2017

I can spend some time doing this, if you feel like it's a good example. My only concern is that I haven't used it enough -- e.g. it's not battle tested. However, I am currently actively working on an app where I use this pattern extensively and hopefully soon will either gain more confidence in it or learn its flaws.

@timojaask

This comment has been minimized.

Copy link

commented Jul 8, 2017

@DivineDominion I've found an annoying issue with this method. With the current implementation, you'd have to create a new enum for each new asynchronous action:

enum FetchPostsState {
    case none
    case request
    case success(posts: [Post])
    case error(error: Error)
}
enum FetchUsersState {
    case none
    case request
    case success(users: [User])
    case error(error: Error)
}
enum FetchWhateverState {
    case none
    case request
    case success(whatever: Whatever)
    case error(error: Error)
}

struct FetchPosts: Action { let state: FetchPostsState }
struct FetchUsers: Action { let state: FetchUsersState }
struct FetchWhatever: Action { let state: FetchWhateverState }

This will eventually be possible to solve using generics when SE-0143 gets released at some point with Swift 4.x. Then we'd be able to create a generic asynchronous request state enum, which would be used with every asynchronous action:

enum AsyncRequestState<T> {
    case none
    case request
    case success(result: T)
    case error(error: Error)
}

struct FetchPosts: Action { let state: AsyncRequestState<[Post]> }
struct FetchUsers: Action { let state: AsyncRequestState<[User]> }
struct FetchWhatever: Action { let state: AsyncRequestState<Whatever> }
@timojaask

This comment has been minimized.

Copy link

commented Jul 18, 2017

@DivineDominion I think I've made a better pattern to handle async operations that my previous proposition. Here's an example project that implements it and includes tests: https://github.com/timojaask/ReSwiftAsyncMiddlewarePattern

The idea is that instead of using a store listener to fire off async requests, you would use a ReSwift middleware. I guess it's kinda like Redux-Saga, but less sophisticated.

This has several advantages over the previous method:

  • Actions are very simple.
  • Code that calls async operations is just swift functions.
  • Application state no longer needs to hold async call state.

Example:

// AppState.swift
// Simple application data state, no "fetchState" stuff
struct AppState: StateType {
    var users: [User] = []
    var posts: [Post] = []
}

// Actions.swift
// Asynchronous actions are simple enums describing the lifecycle of an async operation
enum FetchUsers: Action {
    case request
    case success(users: [User])
    case failure(error: Error)
}

enum FetchPosts: Action {
    case request
    case success(posts: [Post])
    case failure(error: Error)
}

// AppReducer.swift
// Just calling sub-reducers here
func appReducer(action: Action, state: AppState?) -> AppState {
    return AppState(
        users: usersReducer(action: action, state: state?.users),
        posts: postsReducer(action: action, state: state?.posts)
    )
}

// UserReducer.swift
// Updates the User array with new users if FetchUsers.success action is received
func usersReducer(action: Action, state: [User]?) -> [User] {
    let state = state ?? []

    guard let action = action as? FetchUsers,
        case .success(let fetchedUsers) = action else {
            return state
    }

    return fetchedUsers
}

// FetchUsers.swift
// The middleware piece that actually dispatches an async operation
func fetchUsers(dataService: DataService) -> MiddlewareItem {
    return { (action: Action, dispatch: @escaping DispatchFunction) in
        guard let action = action as? FetchUsers,
            case .request = action else { return }

        dataService.fetchUsers()
            .then { dispatch(FetchUsers.success(users: $0)) }
            .catch { dispatch(FetchUsers.failure(error: $0)) }
    }
}

// AppDelegate.swift or some other file
// Register all of your async handlers and pass them to the store as a middleware.
let sideEffects = [
    fetchUsers(RemoteDataService()),
    fetchPosts(RemoteDataService()),
    createPost(RemoteDataService()),
]
let middleware = createMiddleware(items: sideEffects)
let store = Store<AppState>(reducer: appReducer, state: nil, middleware: [middleware])
@rodrigoruiz

This comment has been minimized.

Copy link

commented Aug 6, 2017

@timojaask I'm not sure how that differs from putting the side effects in the action creators. Isn't that what they are for?
I mean, in JS, sure, they are necessary to make sure you're always creating the same action, but that's not the case in Swift, since it's strongly typed.

Besides, in your example (including your ReSwiftAsyncMiddlewarePattern) you're not handling the loading state, i.e., a way to show some UI while loading.

Maybe I misunderstood your approach, but from my perspective, you're just moving action creator code to middlewares, which is ok, but then action creators become unnecessary, so you're just replacing one thing with another. What's the point?

@hlineholm-flir

This comment has been minimized.

Copy link

commented Sep 8, 2017

@rodrigoruiz action creators could be used for async calls or other side effects but thats not their main purpose and I wouldn't do that either as it makes them harder to test, hides debug information from the Redux flow and requires repetitive changes through out your code when you change how for instance errors should be handled.

The main purpose of an action creator is to abstract the creation of your actions. Instead of hard coding your actions everywhere you want to use it, you let your action creator create the action with 0 or more parameters.

Putting generalised logic for handling API requests in a couple of middleware is probably the way to go.

@rodrigoruiz

This comment has been minimized.

Copy link

commented Sep 8, 2017

Don't we have init methods for that purpose?

@DivineDominion

This comment has been minimized.

Copy link
Contributor

commented Sep 28, 2017

@rodrigoruiz Yeah, for synchronous action creation, sure. I figure they are useful when you curry functions like func peelBananaAction(bananaService: BananaService) -> ActionCreator to inject dependencies -- but why not just use the dependency directly and then create the action directly? Dunno.

@timojaask I was thinking about this some more today as I tried to figure out how I can clear a cache (service object outside the State module's scope) when some kind of action passes through.

The approach is similar to your MiddlewareItem stuff. (See http://cleancocoa.com/posts/2017/09/reswift-middleware-different-target/ -- feedback here is very welcome!)

(Though I see its appeal, I don't really like how in the example of struct FetchPosts: Action { let state: AsyncRequestState<[Post]> } the 4-step progress is a property of 1 action. But that's probably just taste -- I'd prefer actions that read like FetchingPosts (resulting in a "loading" state by a reducer, but not dispatching a FetchingPostsIsLoading action or similar) , CompletingFetchingPosts, and an all-purpose HandlingError that gets populated with fetch failure details. Your approach scales better in terms of writing less code, though.)

In your example, how would the Promise survive the autoreleasepool?

    dataService.fetchUsers() // instance not references strongly anywhere
        .then { dispatch(FetchUsers.success(users: $0)) }
        .catch { dispatch(FetchUsers.failure(error: $0)) }
@hlineholm

This comment has been minimized.

Copy link

commented Mar 2, 2018

@rodrigoruiz well you have init methods for initialising an action but an action creator could act as an action factory which will generate different actions depending on the arguments you give it.
For further reading; there are some interesting discussions about where to put async calls, in middlewares or in actions creators, in The Complete Redux Book by Boris Dinkevich and Ilya Gelman.

@wojciech-kulik

This comment has been minimized.

Copy link

commented Mar 5, 2019

Here I described my simple implementation to define API Actions. Partially based on The Complete Redux Book. Also you will find there a link to sample swift project with authentication using this approach.

https://wojciechkulik.pl/ios/swift-how-to-handle-network-async-calls-using-redux

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.