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

getState fetch without cache #111

Closed
rottmann opened this issue Feb 5, 2015 · 17 comments
Closed

getState fetch without cache #111

rottmann opened this issue Feb 5, 2015 · 17 comments

Comments

@rottmann
Copy link

rottmann commented Feb 5, 2015

Referred to #80 (see the code examples)

Every time a navigation is visible, it should be loaded from a remote api, without caching.
I want to use the marty fetch method with the nice promise for handling the status (pending, failed, done).

Could getState have a parameter (or exist a solution) that indicates when getState was called
on before initial render, or on store update ?

With that, the store fetch function can be called with cache enabled or disabled.

@jhollingworth
Copy link
Contributor

So, you want to know at what point in the component lifecycle getState was last called?

@rottmann
Copy link
Author

rottmann commented Feb 6, 2015

Yes, or did you see an other solution?

@jhollingworth
Copy link
Contributor

Hmm, not certain if I see the use case. I've just responded to the previous thread, not sure if that helps at all?

@rottmann
Copy link
Author

rottmann commented Feb 6, 2015

I use a similar idea at the moment, set once a timeout in componentDidMount which reset this.state.cache variable (not so nice but worked for me).

Cleaner would be if this.fetch without locally would automatically fetch remotely.

@jhollingworth
Copy link
Contributor

The problem with this approach is the component is responsible for determining when the state is re-fetched. If a component is listening to multiple stores then a change to a different store could inadvertently cause the state to be re-fetched. Any changes to the state should come from an action creator, the fetch should just be responsible for encapsulating the fact that state might not be there yet.

That said, locallydefaults to noop so if you really want this behaviour you just have to not implement locally.

@rottmann
Copy link
Author

rottmann commented Feb 6, 2015

Found a simple solution, completely store based:

Cache the result until it was loaded (done), on the the next getState -> getAllProjects call, it fetch remotely again.

    getInitialState: function () {
        return {
            cache: false,
            projects: []
        };
    },

    getAllProjects: function () {
        var result = this.fetch({
            id: 'all-projects',
            locally: function () {
                if (this.state.cache === false)
                    return undefined;

                if (this.hasAlreadyFetched('all-projects'))
                    return this.state.projects;
            },
            remotely: function () {
                self.state.cache = true;
                return ProjectApi.loadProjects();
            }
        });

        // disable cache on done / error
        if ( ! result.pending)
            this.state.cache = false;

        return result;
    }

@prayerslayer
Copy link

That said, locally defaults to noop so if you really want this behaviour you just have to not implement locally.

Is that really the case? From my understanding the Fetch API works as follows:

  1. Call locally().
  2. If it returned undefined (just as by calling a noop) call remotely() to get the state into the local cache.
  3. Call locally() again under the assumption that the state is now locally available.
  4. If it returned undefined again (because noop) mark the FetchResult as failed. For good reason, btw.

So it would never work by not implementing locally(). Also: locally is listed as required in the docs.

I for one would be very happy if we could get this behavior without @rottmann`s workaround. ^_^

@rottmann
Copy link
Author

I did not use the Fetch API and the workaround anymore, replaced it by call an action in componentWillMount that load the ajax data with superagent.
It feels more "the flux way" and keep the stores clean.

@oliverwoodings
Copy link
Contributor

@rottmann I disagree on that being more 'flux' - your views should not be directly responsible for triggering data loading. What if in the future you create another separate component that also needs the same data? You then have an issue whereby either two components trigger a data load, or you have to somehow make one component dependent on the other just to pass the data around. Stores should be a 'single source of truth'.

@AlanVerbner
Copy link

@rottmann Why did you leave the workaround you proposed? Is there any problem with it that i'm not seeing besides it needs to be coded on each store?

@FoxxMD
Copy link

FoxxMD commented Mar 16, 2015

@oliverwoodings Wouldn't it still be a 'single source of truth' if the data loaded by the action in @rottmann's method is still put into the store? I am using a similar approach for some reports in part of my app where I want the data to be loaded every time a user see a certain component. I'm achieving this while still maintaining what I consider good flux practices by:

  1. Action triggered in componentWillMount
  2. Action calls state source method for reports store that gets data for that report
  3. State source follows up with loaded data with source action creator that loads data into store with update method constant (handled by store using report-specific id passed from source)

In the component I use a store mixin that get uses a store method that does not use locally/remotely but gets based on the report-specific id -- initial state in the store in undefined. That way the store always has the 'single source of truth' but every time the component is mounted I know I will get fresh data.

In retrospect I guess I could use hasAlreadyFetched and forego the action in componentWillMount but I find dealing with fetch is a pain when I'm fine with initial state being undefined until a component re-renders after the store updates.

@oliverwoodings
Copy link
Contributor

@FoxxMD see the end of my last comment:

What if in the future you create another separate component that also needs the same data? You then have an issue whereby either two components trigger a data load in componentWillMount, or you have to somehow make one component dependent on the other just to pass the data around. Stores should be a 'single source of truth'.

It is fine for the initial state of the store to be undefined - that is what fetch is designed to manage. If you want to re-load the data whenever your component remounts you should dispatch an action in componentWillMount that causes the store to reset, something like this: Your store should be something like this:

var ReportStore = Marty.createStore({
  handlers: {
    setReports: Constants.REPORTS_LOADED,
    clearReports: Constants.CLEAR_REPORTS
  },
  loadReports: function (reports) {
    this.state = reports;
  },
  clearReports: function () {
    this.state = [];
  },
  getInitialState: function () {
    return {};
  },
  getReport: function (id) {
    return this.fetch({
      locally: function () {
        return this.state[id];
      },
      remotely: function () {
        return ReportAPI.getReports();
      }
    });
  }
});

And your component like this:

var ReportState = Marty.createStateMixin({
  listenTo: [ReportStore],
  getState: function () {
    return ReportStore.getReport(reportId)
  }
});
var Report = React.createClass({
  mixins: [ReportState],
  componentWillMount: function () {
    ReportActions.clearReports();
  },
  render: function () {
    this.state.when({
      pending: function () {
        return <div>Loading...</div>;
      },
      done: function (report) {
        return <div>{JSON.stringify(report)}</div>;
      },
      failed: function (err) {
        return <div>Sumthin fuccky</div>;
      }
    });
  }
});

@FoxxMD
Copy link

FoxxMD commented Mar 16, 2015

In your example wouldn't the components still be dependent on each other since they are relying on one of them to clear the data?

@oliverwoodings
Copy link
Contributor

No, because it doesn't matter if all of them clear the data

@idolizesc
Copy link

I still think this would be extremely useful to at least have an example of how to perform fetches remotely with no local cache.

@taion
Copy link
Member

taion commented Jul 2, 2015

I would rephrase that - you always need some sort of local cache, because the data rendered in a component needs to live in a store. What you specifically want is to refresh the data on the initial mount of a component, which is a little different, and requires something on the container API.

@taion
Copy link
Member

taion commented Jul 2, 2015

ref #354

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

8 participants