Batching of in-flight requests

[ericclemmons]

Batching of in-flight requests

[ericclemmons]

Correct. I'm having the exact same thing happen in a project, and we opted for a Store.

The reasoning being, the Resolver simply doesn't know which data is being used in the request. There could be an external variable (outside of props or context) that's being incremented.

Unfortunately, I haven't heard of any progress in axios/axios#31.

The Store solution worked out best (in my testing today), since it was easiest to cache the request there and all current & future Promises receive the same payload.

Plus, I don't think this would work with your earlier request to support callbacks (#35), would it?

[goatslacker]

I've created a really naive proof of concept:

import axios from 'axios'

const inFlightRequests = {}

function source(config) {
  const id = JSON.stringify(config)

  if (inFlightRequests[id]) {
    return inFlightRequests[id]
  }

  return new Promise((resolve, reject) => {
    inFlightRequests[id] = axios(config)
    inFlightRequests[id].then(resolve).catch(reject)
  }).then(value => {
    delete inFlightRequests[id]
    return value
  }).catch(error => {
    delete inFlightRequests[id]
    return error
  })
}

export default source

[ericclemmons]

So, you agree that this is better suited for a Store/Service since the Resolver doesn't know what's going on in the async prop's "blackbox"?

[goatslacker]

Oh yeah totally this shouldn't exist in the resolving part but I do think this should be included as a part of react-resolver somehow somewhere.

[ericclemmons]

What do you think of having something like:

import { memoize } from "react-resolver";
// or `cache` or `cacheable` or whatever
...
resolve: {
  user: memoize(function(props, context) {
    return whatever;
  }
}

But it can act like _.memoize where it does a shallow compare (===) of props & context.

Alternatively, the signature can be:

memoize(resolver(props, context)[, keyGenerator(props, context)]);

So, to always cache you can do:

user: cache(function(props, context) {
  return ...;
}, () => true);

Or...

user: cache(function(props, context) {
  return ...;
}, () => props.userId);

"But wait! props.userId will collide with other cache keys!"

Not so, I say! The key should be computed with the container's ID and prop, so in effect it'll be .0.1.2.3.4-user-123 that holds the cached value.

[goatslacker]

Batching and caching are separate issues. Caching is definitely on my todo list and I'd love to see it used with resolver.

I do have concerns about including it here though.

• we will have to support or build a good caching algorithm.
• we will need to have some way to hook this into flux stores. I feel like caching makes a lot of sense for a flux store.

Alternatively you can go the relay route and provide a single store as part of this project. Perhaps it can be a baobab tree?

[ericclemmons]

I built this today, and it seems to make the most sense within a store, or as part of the HTTP library, not the Resolver:

import axios from "axios";
import Actions from "./Actions";

export default class HttpActions extends Actions {
  initialize() {
    this.requests = {};
  }

  get(url, params) {
    return this.dispatcher.actions.Http.request("get", url, params);
  }

  cache(request, signature) {
    const key = JSON.stringify(signature);

    this.requests[key] = request;

    return request;
  }

  cached(signature) {
    const key = JSON.stringify(signature);

    return this.requests[key];
  }

  request(method, url, params) {
    const { cache } = this.dispatcher.actions.Http;
    const existing = this.cached(arguments);

    if (existing) {
      return existing;
    }

    const request = axios({
      method,
      url,
      [method === "get" ? "params" : "data"]: params,
    }).then(response => response.data);

    return cache(request, arguments);
  }

  post(url, data) {
    return this.dispatcher.actions.Http.request("post", url, data);
  }
}

[goatslacker]

Yes I agree this should be part of the http library. What I'm proposing here is to optionally include an http library that supports this so this can be a "complete" solution.

I say optional because people may want to change out their http library.
And I say complete because batching requests is a must have for something like this.

[ericclemmons]

@goatslacker Man, count me confused. Each HTTP library is so similar and yet so different that it's a beast I'd prefer for this library to not even tackle.

I mean, my examples yesterday with Form.load(...) branching to Form.create the first time & Form.get thereafter removes this need.

You argued in #15 to have a dependency-less build, and I think that still makes a lot of sense.

I'd rather see effort expended towards other libraries (axios/axios#31) than ship a custom HTTP implementation =/