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

Force fetch remotely #80

Closed
bigardone opened this issue Jan 25, 2015 · 13 comments
Closed

Force fetch remotely #80

bigardone opened this issue Jan 25, 2015 · 13 comments

Comments

@bigardone
Copy link
Contributor

Hi!
I'm starting to use marty and I have a problem. My application has a PeopleStore and a PeopleAPI which returns some paginated list of json results.
So in my main component view I display those first results and a pagination component which just handles the on click event on any of it's page links, so when they are clicked it tries to fetch again the data from the server for the selected page results...
Te problem is that in my store it always returns the already loaded results from the first time as it always fetches locally first... is there a way of forcing it to fetch again remotely from the server?

Here's my code:

@PeopleStore = Marty.createStore
  displayName: 'PeopleStore'

  getInitialState: ->
    meta:
      totalPages: 0
      currentPage: 1

  handlers:
    receivePeople: PeopleConstants.RECEIVE_PEOPLE
    findPeople: PeopleConstants.FIND_PEOPLE

  findPeople: (params) ->
    @.fetch
      id: 'all-people'
      locally: () ->
        @state.people
      remotely: () ->
        PeopleAPI.findPeople(params)

  paginationMeta: ->
    @state.meta

  receivePeople: (response) ->
    @setState
      people: response.people
      meta: response.meta

@PeopleAPI = Marty.createStateSource
  type: 'http'

  findPeople: (params) ->
    searchText = if params and params.searchText then params.searchText else ''
    pageNumber = if params and params.pageNumber then params.pageNumber else 1

    req =
      url: Routes.people_path(
        search: searchText
        page: pageNumber
      )

    @get(req).then (res) ->
      PeopleSourceActionCreators.receivePeople res.body

I have a full demo, only using React components, of what I'm trying to achieve with using marty.js here's the link http://rails-and-react-iii.herokuapp.com/

Thank you very much in advance!

@jhollingworth
Copy link
Contributor

Hey, good question. remotely will be called if locally returns undefined (or it doesn't return anything). So if your store keeps track of the current page you could do something like this:

@.fetch
  id: 'all-people'
  locally: () ->
    return @state.people if @state.meta.currentPage == pageNumber
  remotely: () ->
    PeopleAPI.findPeople(params)

I'm wondering, would it be worth caching pages of users locally? Something like this

@PeopleStore = Marty.createStore
  getInitialState: ->
    people: []
    meta:
      totalPages: 0
      pagesLoaded: {}

  findPeople: (params) ->
    pageNumber = if params and params.pageNumber then params.pageNumber else 1

    @.fetch
      id: 'page-#{pageNumber}'
      locally: () ->
        if @state.meta.pagesLoaded[pageNumber]
          return _.where(@state.people, pageNumber: pageNumber)
      remotely: () ->
        PeopleAPI.findPeople(params)

  receivePeople: (response) ->
    @state.totalPages = response.meta.totalPages
    @state.pagesLoaded[response.meta.page] = true
    @state.people = @state.people.concat(response.people)
    @.hasChanged()

@bigardone
Copy link
Contributor Author

Thanks @jhollingworth for your response, it totally make sense, now I've got it working! :)

@nhagen
Copy link

nhagen commented Feb 3, 2015

So in this example, page change invalidated the catch which is convenient.

What if you have a master list that you always want to reflect the latest snapshot of the database? For instance, if I have a view where i always want to invalidate the local? Nothing is changing with the meta state of the app, only the data is being updated.

I'm running into an infinite loop issue because I'm using the state mixin, which executes the getAll store method every time theres a change to the store (which is every time that I call getAll). Is the only solution to just not use the state mixin, and to not subscribe to changes? How could I use the fetch state (pending, etc) in the view in that case?

var OrderStateMixin = Marty.createStateMixin({
  listenTo: OrderStore,
  getState: function() { return { orders: OrderStore.getAllOrders({ cache: false });
});

var OrderStore = Marty.createStore({
  getAllOrders: function(options) {
    options = options || { cache: true };
    return this.fetch({
      id: 'GET_ALL_ORDERS',
      locally: function() {
         return !options.cache ? this.state.orders : undefined;
      },
      remotely: function() {
         return OrderApi.getAllOrders();
      }
    });
  },

  receiveOrders: function(orders) {
    this.setState({ orders: orders });
  }
});

@bigardone
Copy link
Contributor Author

Hi @nhagen
Maybe you can use a state attribute of the store to control that, something like:

    var OrderStore = Marty.createStore({
      getAllOrders: function(options) {
        options = options || { cache: false };

        this.state.cache = options.cache;

        return this.fetch({
          id: 'GET_ALL_ORDERS',
          locally: function() {
             return this.state.cache ? this.state.orders : undefined;
          },
          remotely: function() {
             return OrderApi.getAllOrders();
          }
        });
      },
      receiveOrders: function(orders) {
        this.setState({ orders: orders, cache: true });
      }
    });

So every time it fetches for the first time locally it returns undefined forcing fetching remotely, which sets then sets cache to true so the second local fetch will return the state projects. The next call to projects from the mixin will pass cache equal false starting all over again.

Hope this helps :)

@rottmann
Copy link

rottmann commented Feb 4, 2015

@bigardone the problem from @nhagen (and currently for me too) is e.g. a rendering with pending and done output.

When you fetch remotely, render will be called with promise status pending, but before every render the getState will be called again with cache: false, which ends in an infinite loop.

The only hacky solution is a cache timeout after some seconds. Or am I missing something?

@jhollingworth
Copy link
Contributor

@nhagen

The fetch API is designed for the case when you expect some state to be there but its not there right now. If I understand you correctly, you have some state in the store but its stale. If thats the case, I think you should have an action which is responsible for refreshing the state.

e.g.

var OrderActionCreators = Marty.createActionCreators({
  refreshOrders: Constants.REFRESH_ORDERS(function () {
    OrderAPI.updateAllOrders();
  });
})

setInterval(OrderActionCreators.refreshOrders, 100);

@nhagen
Copy link

nhagen commented Feb 8, 2015

Yup, that worked great. I call OrderActionCreators.refreshOrders on getInitialState in my store, and in OrderActionCreators.refreshOrders I have

var timeout;
var OrderAction ...
  refreshOrders: ...
    clearTimeout(timeout); 
    timeout = setTimeout(this.refreshOrders, 10000)

which sort of resets the timeout loop every time the user chooses to manually refresh. An interval of course would work just as well.

Having the refresh as an action ads a lot of flexibility. You can have user-level auto-refresh based on their user settings, or you can have a manual refresh button, or you can do a combination of the two. I think this is better UX and is less bug prone than having refreshes happen behind the scenes and outside of the normal Flux flow.

@grmlin
Copy link

grmlin commented May 8, 2015

I have to revive this issue, because I'm not sure if I use fetch correctly at the moment.

I love how marty containers handle all the fetch/loading code for me, so I want to use it. Anyway, when I'm navigating my app without page reloads I have to fetch the data remotely when a page opens. With fetch this is not possible, because it will be cached once and forever.

Right now I have a parent component in which I invalidate the cache so it's fetched remotely, again. My problem is, I have no idea how to do this in the container using the store itself.

What I want to do is this:

Marty.createContainer(ProfileShow, {
    componentWillMount(){
        UserStore.invalidateCache();
    },
    listenTo: UserStore,
    fetch   : {
        user() {
            return UserStore.for(this).getUser();
        }
    },
    pending() {
        return <div className='pending'>Loading user data...</div>;
    },
    failed(errors) {
        return <div className='error'>Failed to load user. {errors}</div>;
    }
});

The code above does not work as intended, because the componentWillMount method is called after the first fetch is made. Is there any other hook I can use to invalidate the cache before the fetch is done initially? Setting timeouts to invalidate the cache does not feel good, too.

It's a bit cumbersome to wrap everything with components so that I can force a remote fetch.

Thanks a lot for your help!

@taion
Copy link
Member

taion commented May 8, 2015

BTW, in general you should not be directly mutating stores except via actions.

Why not just invalidate the cache when your component unmounts? Alternatively, keep track of whether or not this is the first fetch you've done, and invalidate the cache if so.

@taion
Copy link
Member

taion commented May 8, 2015

An additional thing is that I've been using a strategy like the one proposed here: #80 (comment) where I force a remote fetch on component load, without explicitly invalidating what's currently in the cache.

@grmlin
Copy link

grmlin commented May 9, 2015

You are right. I directly mutated the store to simplify the example.

Anyway, on unmount will work in my case. Thanks! It will not work if the component remains and another one using this store is mounted. But this would be acceptable I think, if it ever happens...

I will look into your comment, too. I always ended in some sort of endless loop because the fetch methods gets called multiple times.

@taion
Copy link
Member

taion commented May 9, 2015

I don't think the on-mount behavior you have is particularly well defined when you have multiple components accessing the same data either.

To get refresh working without infinite loops, you have to write it like:

  getData(id) {
    return this.fetchData(id, function () {
      return this.state[id];
    });
  }

  refreshData(id) {
    let refresh = true;

    return this.fetchData(id, function () {
      if (refresh) {
        refresh = false;
        return undefined;
      } else {
        return this.state[id];
      }
    });
  }

  fetchData(id, locally) {
    return this.fetch({
      id,
      locally,
      remotely() {
        return DataQueries.getData(id);
      }
    })
  }

@grmlin
Copy link

grmlin commented May 9, 2015

Well, I think when I use multiple components on a single page requesting the same data over and over again, my app structure is questionable anyway :)

Well, I did it sort of the way you describe above in the first place, but without fetch, locally and remotely because it added so much additional code ;)

I think I know what to do now, and how to add a refresh functionality. Thanks for your help!

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

6 participants