alt icon indicating copy to clipboard operation
alt copied to clipboard

StoreMixin: Seems impossible to handle an error without Storemixin throwing it in any case

Open SachaMPS opened this issue 9 years ago • 9 comments

We're using static function in stores to trigger xhr calls - as far as i can see they are defined correctly. However it seems impossible to avoid that the Storemixin itself throws an error. As far as i understand the alt-source the following line will always be triggered if an xhr error occurs: https://github.com/goatslacker/alt/blob/9d228dce2295890cb9a0ea114bd5840a128ba55d/src/alt/store/StoreMixin.js#L57

Is this the desired behaviour? I mean to pass in a truthy value in any case? https://github.com/goatslacker/alt/blob/9d228dce2295890cb9a0ea114bd5840a128ba55d/src/alt/store/StoreMixin.js#L70

*** Sample Code for an XHR Handler ***

const DesignSource = (alt) => {
    return {
        rate: {
            remote(state, rating, designId) {
                return api.post('designs/' + designId + '/rate', {
                    rating: rating
                });
            },
            success: alt.getActions(designs').addRating,
            error: alt.getActions('designs').xhrError
        }
    };
};

SachaMPS avatar Jun 23 '15 13:06 SachaMPS

Once #343 gets meged in your problem should be resolved

Jyrno42 avatar Jun 24 '15 15:06 Jyrno42

+1 I'm blocked with this problem too

bolismauro avatar Jul 07 '15 15:07 bolismauro

+1 :wave: is there anything I can do to move this along?

peterpme avatar Sep 30 '15 00:09 peterpme

I'd also love to see this working. It would be very nice to have one place to handle all the errors for stores and data sources.

janmyler avatar Oct 14 '15 12:10 janmyler

@janmyler for that to work you could create an ErrorStore that listens for errors in your other stores. Then create a dictionary that handles status codes/messages and delivers that to your components

peterpme avatar Oct 14 '15 15:10 peterpme

I have something I've been somewhat happy with lately:

const getDispatcher = alt => (action, data) => {
  if (!action) return

  if (action.id && action.dispatch) {
    return alt.dispatch(action, data)
  } else {
    return action(data)
  }
}

class AltRequest {
  constructor(config) {
    this.actions = {}
    this.pending = {}
    this.requests = {}
    this.cache = config.cache

    // store the valid action handlers
    const validHandlers = ['success', 'failure', 'begin', 'end']
    validHandlers.forEach((actionName) => {
      if (config[actionName]) {
        if (!config[actionName].id) {
          throw new Error(`${actionName} must be an action`)
        }
        this.actions[actionName] = config[actionName]
      }
    })

    if (config.store) this.setStore(config.store)
  }

  setStore(store) {
    this.store = store.getInstance
      ? store.getInstance()
      : store
  }

  send(id, behavior) {
    if (typeof id !== 'string') {
      throw new ReferenceError('Each fetch must provide an id')
    }

    // you can pass in a function or an object
    // the function form will default to remote()
    const source = typeof behavior === 'function'
      ? { remote: behavior }
      : behavior

    // extend source with these convenience methods
    source.isLoading = () => this.isLoading(id)
    source.getRequest = () => this.getRequest(id)

    const state = this.store.getState()

    const dispatch = getDispatcher(this.store.alt)

    // if the value is not available then default to checking for a
    // pending request, this automatically de-dupes any in flight requests
    let value = source.local && source.local(state)
    value = value == null ? this.getRequest(id) : value

    // only call shouldFetch if we're not loading a request, this is
    // so a duplicate request isn't sent out
    const shouldFetch = !this.isLoading(id) && source.shouldFetch
      ? source.shouldFetch(state)
      : value == null

    if (shouldFetch) {
      dispatch(this.actions.begin)

      const result = source.remote(state).then((res) => {
        dispatch(this.actions.success, res)
        return res
      }, (err) => {
        dispatch(this.actions.failure, err)
        return Promise.reject(err)
      })

      this.pending[id] = true
      this.requests[id] = result

      result.then(() => {
        dispatch(this.actions.end)
        delete this.pending[id]
        if (!this.cache) delete this.requests[id]
      })
      return result
    } else {
      return Promise.resolve(value)
    }
  }

  isLoading(id) {
    return this.pending.hasOwnProperty(id)
  }

  getRequest(id) {
    return this.requests[id]
  }
}

export default AltRequest

Usage

import AltRequest from '../utils/AltRequest'
import UserActions from '../actions/UserActions'

export default (stores, alt) => {
  const request = new AltRequest({
    store: stores.UserStore,
    success: UserActions.authorizedUser,
    failure: UserActions.userNotAuthorized,
  })

  return () => {
    return request.send('user', {
      local: state => state.user,
      shouldFetch: state => state.user === undefined,
      remote: () => axios.get('/user'),
    })
  }
}

This completely replaces data sources, it's very similar to it however. Main difference is that its decoupled from stores, doesn't throw when it encounters an error (it rejects the Promise), and has methods for isLoading() and for caching the request.

goatslacker avatar Oct 14 '15 21:10 goatslacker

@goatslacker that looks quite interesting. Thanks for sharing!

janmyler avatar Oct 20 '15 19:10 janmyler

Got more code to share

Instead of classes you get functions, less boilerplate-y too since it auto generates the async actions for you.

import isPromise from 'is-promise'


// testing imports
import Alt from 'alt'
// //

const dispatch = (data = null) => data

// get the action creators that will be created given a namespace
export const getActionCreators = (namespace) => {
  return {
    begin: {
      id: `${namespace}/begin`,
      dispatch,
    },
    success: {
      id: `${namespace}/success`,
      dispatch,
    },
    failure: {
      id: `${namespace}/failure`,
      dispatch,
    },
    end: {
      id: `${namespace}/end`,
      dispatch,
    },
  }
}

// convenience function so you can use function call form instead of creating
// the object yourself
export const send = (id, value) => {
  return { id, value }
}

// convenience function so you can use this with alt
export const dispatchWithAlt = alt => action => action({
  dispatch(res) {
    return alt.dispatch({
      id: res.type,
      dispatch: () => res.payload,
    })
  },

  alt,

  stores: alt.stores,
})

// Creates a function that when called with a dispatcher will
// create all the necessary actions for you and dispatch them in the correct
// order. Basic caching is also provided by the utility
const createAsyncDispatch = (namespace, fn, opts) => {
  const { begin, success, failure, end } = getActionCreators(namespace)

  const store = { pending: {}, requests: {} }

  return (...args) => dispatcher => {
    const { id, value } = fn(...args)

    const asyncDispatch = (value) => {
      dispatcher.dispatch({
        type: begin.id,
        payload: id,
      })
      const result = value.then((res) => {
        dispatcher.dispatch({
          type: success.id,
          payload: res,
        })
        return res
      }, (err) => {
        dispatcher.dispatch({
          type: failure.id,
          payload: err,
          error: true,
        })
        return Promise.reject(err)
      })

      store.pending[id] = true
      store.requests[id] = result

      value.then(() => {
        delete store.pending[id]
        if (!opts.cache) delete store.requests[id]
        dispatcher.dispatch({
          type: end.id,
          payload: id,
        })
      })
      return result
    }

    const result = value({
      state: opts.getState ? opts.getState(dispatcher) : null,
      isLoading: () => store.pending.hasOwnProperty(id),
      getRequest: () => store.requests[id],
    }, dispatcher)

    // remote request
    if (isPromise(result)) {
      return asyncDispatch(result)

    // a function, handle your own async dispatches
    } else if (typeof result === 'function') {
      return result(asyncDispatch)
    }

    // local value
    return result
  }
}

export default createAsyncDispatch




// test

const alt = new Alt()

const CompanyStore = alt.createStore({
  displayName: 'CompanyStore',
  state: { x: 'hello world', },
})

const fetchCompany = createAsyncDispatch('company', (companyId) => {
  // !!! you can return this object, or you can use send(id, req => {})
  return {
    id: `companyId/${companyId}`,
    value(req) {
      // if its a value then its local...
      // if its a promise, then its remote
      // if you want both then return a function?
      return new Promise((resolve, reject) => {
        setTimeout(() => {
          resolve(companyId)
        }, 1000)
      })
      return Promise.resolve(companyId)
    },
  }
}, {
  cache: true,
  getState: ({ stores }) => stores.CompanyStore.getState(),
})

//alt.dispatcher.register(x => console.log('Dispatch:', x.action))

const ar = dispatchWithAlt(alt)

ar(fetchCompany(123))
ar(fetchCompany(123))

goatslacker avatar Oct 21 '15 08:10 goatslacker

Hi @goatslacker, got back to this thread after some time and played around with the last code example you've posted (once again, thanks for sharing!).

I've noticed because this solution is Alt independent, the action creators – unlike the ones created using alt.createActions – do not receive the ‘namespace’. I mean, if I create action creators the Alt way, they have a namespace of the class (e.g. UserActions.fetchUser).

Or maybe I'm just looking for a problem where there's none and simply using getActionCreators('UserActions.fetchUser') do the exactly same thing?

janmyler avatar Mar 23 '16 20:03 janmyler