ReSift icon indicating copy to clipboard operation
ReSift copied to clipboard

Allow more specification to the loading status behavior

Open ricokahler opened this issue 5 years ago • 2 comments

@pearlzhuzeng had experienced an unexpected result regarding the LOADING status when developing the ReSift Rentals demo.

The demo is application is structured like Netflix's list of lists of movies. She had a few fetches that we were shared and they were also using the newer "merges across namespaces" feature.

The issue and question is: what is the status of a fetch that has a share across a namespace?

Here's the scenario, you have three fetches with two different namespaces:

  • makeGetGenre - a fetch factory that gets a list of movies. namespace: genre
  • makeGetMovie - a fetch factory that gets a single movie. namespace: movie
  • makeUpdateMovie - a fetch factory that updates a single movie. namespace: movie

makeGetGenre implements a merge for the namespace movie so that when a movie item changes, the genre list changes to reflect that change.

The question is, what is the status of a genre when a movie is inflight?

// after you dispatch an update...
const updateMovie = makeUpdateMovie(movieId);
dispatch(updateMovie());
// ...what is the status of the genre list?
const getGenre = makeGetGenre(genreId);
const status = useStatus(getGenre);

// what should this be??
console.log(isLoading(status));

Through the current implementation, the output of isLoading is true.

With this behavior, whenever any movie goes inflight, it will cause all lists to have the status LOADING. This is because the genre namespace doesn't know which movie belongs to which genre (and you can't know ahead of time).

This can be confusing to the developer because an update to a movie in a different genre would cause all the genres to update (which is what @pearlzhuzeng experienced). I think this behavior should not be the default and we should add other behavior options.


The proposal is to remove the option isolatedStatus and replace it with type.

The possible types of values of type will be:

  • self - only consider the status of the current fetch factory + key
  • namespace - only consider the statuses of all fetches related the current namespace + key
  • all - consider the statuses of all the fetches related to the current namespace + key as well as any other namespace (e.g. genre namespace and movie).
const selfStatus = useStatus(getGenre, { type: 'self' }); // this is the new `isolatedStatus: true`
const namespaceStatus = useStatus(getGenre, { type: 'namespace' });
const allStatus = useStatus(getGenre, { type: 'all' });

I think the default type for useStatus should be namespace.


That's it! Let me know if you need any further clarification. I think @pearlzhuzeng knows what I'm talking about though.

ricokahler avatar Nov 22 '19 01:11 ricokahler

Interesting concept. I think namespace is what separates resift from other data management libraries.

JulesPatry avatar Nov 26 '19 15:11 JulesPatry

@ricokahler Thank you for the proposal write up!

Personally, I feel that the fetch status for getGenre should be loading if it needs to merge in the change of getMovie whenever a movie within that genre gets updated.

The challenge is that getGenre doesn't know which genre that updated movie belongs to.

So I think figuring out a way to inform getGenre the genreId of that updated movie would be a better route to go. The most straightforward way may be updating makeGetGenre to expose the genreId of that fetch instance in the return of the fetch. Since when we fetch a movie with getMovie, we are also getting a genres array along with the movie object that indicates the genres that movie belongs to, then in the merge block in getGenre, we can check if the the genre id of the intended fetch is included within the genre id array of the movie before committing a merge. Please take a look at the example below, especially the two lines marked by '👈'

const makeGenre = defineFetch({
  displayName: 'genre',
  share: {
    namespace: 'moviesOfGenre',
    merge: {
      moviesOfGenre: (previous, response) => {
        ...
        };
      },
      movie: (previousMovies, incomingMovie) => {
        if (!previousMovies) return null;
        
        if (!incomingMovie.genreIds.includes(previousMovies.genreId)) return null; 👈

        const index = previousMovies.response.results.findIndex(movie => movie.id === incomingMovie.id);

        if (index === -1) {
          return previousMovies;
        }

        return {
          ...previousMovies,
          response: {
           ...previousMovies.response, 
            results: [
              ...previousMovies.results.slice(0, index),
              {
                ...previousMovies.results[index],
                name: incomingMovie.name,
              },
              ...previousMovies.results.slice(index + 1, previousMovies.results.length),
            ],
        }};
      },
    },
  },
  make: genreId => ({
    request: page => async ({ http }) =>
      { 
        const response = await http({
          method: 'GET',
          route: `/genres/${genreId}/movies`,
          query: {
            page,
            pageSize: 10,
          },
        });

        return { response, genreId } 👈
      }
  }),
});

pearlzhuzeng avatar Jan 28 '20 20:01 pearlzhuzeng