alt icon indicating copy to clipboard operation
alt copied to clipboard

Source local result doesn't call success handler on Store

Open nross83 opened this issue 10 years ago • 11 comments
trafficstars

Say I have a cache on my store that looks something like this:

import alt from '../dispatcher';
import MyActions from './MyActions';
import MySource from './MySource';

class MyStore {
  constructor() {
    this.bindListeners({
      handleLoading: MyActions.getResult,
      handleSuccess: MyActions.getResultSuccess,
      handleFailure: MyActions.getResultFailure
    });
    this.resultMap = {}; // cache
    this.currentResult = {}; // object used in my view component
    this.exportAsync(MySource);
  }

  handleLoading() {
    this.currentResult = {};
  }

  handleSuccess(result) {
    this.resultMap[result.id] = result;
    this.currentResult = result;
    this.errorMessage = null;
  }

  handleFailure(error) {
    this.errorMessage = error;
  }
}

export default alt.createStore(MyStore);

and a Source object that looks like this:

import MyActions from './MyActions';

export default {
  fetchResult() {
    return {
      remote(state, id) {
        return request.get(`http://my/service/${ id }`) // returns a promise
      },
      local(state, id) {
        return state.resultMap[id] ? state.resultMap[id] : null;
      },
      loading: MyActions.getResult,
      success: MyActions.getResultSuccess,
      error: MyActions.getResultFailure
    }
  }
}

The first time I call fetchResult, the handleSuccess function is called in the store and I add the response to a cache resultMap. I then use currentResult in the component and render with it. The next time I call fetchResult with the same ID, local would be called and then return the object in the cache.

At this point, I would expect handleSuccess to be called again so the proper state variables would be set (currentResult and resultMap which would essentially be a no-op) before emitChange occurs. Instead, the local function returns the object and emitChange is called internally without giving me the ability to alter the state in the store. As a result, I have a view that doesn't update when I get cached results.

nross83 avatar Jun 16 '15 22:06 nross83

+1 same problem.

In this example, returning a value from local() prevents remote() from being called, but never calls the success action. Works fine with just remote() defined.

export default {
  getTemperature: {
    remote(state, zip) {
      return fetchTemperature(zip);
    },

    local() {
      return 98;
    },

    success: WeatherActions.loadTemperature,
    error: WeatherActions.temperatureError,
    loading: WeatherActions.loadingTemperature
  }
};

Maybe I'm missing something and the way for to listen to local changes is different?

ddgromit avatar Jun 17 '15 08:06 ddgromit

Based on the source, it looks like the success action only gets called when the remote data is fetched. It only emits a change when the local data returns something (as seen here), rather than calling the success action too.

I am guessing that it was assumed that the success action handles setting of data, and does not need to be called again if local returns something. I think the use case for it calling the success action definitely exists based on your examples.

We can probably just replace the emit change in the else statement with makeActionHandler(spec.success) to get the desired behavior. The thing I am not sure about is that if people will want the success action to always be called even if remote is not called. I think we should also add a config to specify whether it is desired or not.

jdlehman avatar Jun 21 '15 15:06 jdlehman

Just ran into this problem. This is definitely a major issue/blocker, as we shouldn't resort to constantly fetching the data.

milesj avatar Jun 24 '15 23:06 milesj

I ran into the same issue as well. Curious if there is a workaround or if this is intended?

I tried updating the store's state from local() hoping that update the view but it doesn't appear to do that from what I could see.

Any suggestions?

jwdotjs avatar Jul 03 '15 13:07 jwdotjs

I ran into this as well. At first I expected it to call onSuccess, so when fetching a specific user profile i tried to return that from local.

Looking at the search example in the docs and thinking about it a bit more, it seems like not calling onSuccess is the intention and is the correct way to do things. Presumably, onSuccess puts something in the store's cache and then emits a change. With local, you've determined that something has already been cached in onSuccess, so calling onSuccess again would be redundant.

I set up a public function that tries to grab the profile from the cache on the store which I use in getPropsFromStores, then call fetch in componentWillMount if that value isn't there.

I was curious if there was a better way though because it did seem a little confusing. On Fri, Jul 3, 2015 at 6:01 AM Jason Walsh [email protected] wrote:

I ran into the same issue as well. Curious if there is a workaround or if this is intended?

I tried updating the store's state from local() hoping that update the view but it doesn't appear to do that from what I could see.

Any suggestions?

— Reply to this email directly or view it on GitHub https://github.com/goatslacker/alt/issues/335#issuecomment-118342440.

mwildehahn avatar Jul 03 '15 15:07 mwildehahn

I've had a similar experience in my Typescript port of alt-tutorial.

remote() {
              console.warn("Remote");
              return new Promise<Array<Location>>((res, rej) => {
                  setTimeout(() => {
                      if(true) {
                          res(mockData);
                      } else {
                          rej("Things have broken");
                      }
                  }, 250)
              })
          },
          local(state):Location[] {
              console.warn("Local");
              console.warn(state);
              //TODO : Figure out why local doesn't work =(
              return mockData;
          },
          success:locationActions.updateLocations,
          error:locationActions.locationsFailed,
          loading:locationActions.fetchLocations,
          shouldFetch:() => true

If I don't set should fetch to always return true the app will never grab its initial state even though it seems like it should be returned by local.

Shearerbeard avatar Jul 03 '15 15:07 Shearerbeard

We just started having similar problems. For us it appears to help to remove the loading actions. The loading logic may be failing and preventing our local/shouldFetch from being honored? It might be something else for folks to try, anyway. We'll continue our debugging.

geekytime avatar Jul 09 '15 20:07 geekytime

possible workaround:

local(){
  let data = localStorage.getItem('favorites');

  if (data) {
    // skip current dispatch and call success callback manually
    setTimeout(this.success.bind(this, data));
    return data;
  }
}

gbiryukov avatar Jan 02 '16 13:01 gbiryukov

When you retrieve data using the remote method in your source, you will ultimately have the success method in your store called, On success, store the value retrieved in a cache in your store. You can look up data from the cache by id. In your local method of your source, when attempting to get data, check if the id requested is a key in the cache already, if so local should return true. Then remote is not called. Your component should always use data from the cache in your store.

bmhardy avatar Jul 07 '16 21:07 bmhardy

Any planned resolution for this?

https://github.com/goatslacker/alt/blob/ca445de74e3473ace9734e57662f56ab68fb368d/docs/async.md#success

Based on the Sources docs, it appears that intended behavior is for success to be called after the resolution of local or remote.

sircommitsalot avatar Aug 10 '16 21:08 sircommitsalot

I just ran into this as an issue. Seems strange that a local retrieve isn't firing success. I just put my local resolution inside remote for now.

hsimah avatar Mar 17 '18 04:03 hsimah