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

Resolve multiple props with one callback #98

Open
nettofarah opened this issue Oct 29, 2015 · 5 comments
Open

Resolve multiple props with one callback #98

nettofarah opened this issue Oct 29, 2015 · 5 comments

Comments

@nettofarah
Copy link

I got into this situation where my React component requires more than one prop.
One way to solve it was through adding two @resolve calls with two different callbacks.

@resolve("propA", function() { remoteCall() .... return response.propA; })
@resolve("propB", function() { remoteCall().... return response.propB; })

However, I'd like to be able to collect multiple props in one single callback, and hence save one remote call.

I hacked something together that allows me to do something like:

@resolve("{propA, propB}", function() { 
     remoteCall() ... 
     return { response.propA, response.propB };
})
  • Does this make sense?
  • Is there any other way to do this?
  • Does this go against react-resolver's philosophy?

@ericclemmons: I can put together a Pull Request if you think this makes sense.

@ericclemmons
Copy link
Owner

Why not do:

@resolve({
  propA, function() { ... },
  propB, function() { ... }
})

That's what's happening internally:

const asyncProps = (arguments.length === 1) ? prop : { [prop]: promise };

💩 on me for not having this in the README or docs.

@ericclemmons
Copy link
Owner

Misread it. You're right, there's not a way to return an object for multiple props from a single call.

I think this makes sense, especially for something like:

@resolve(function(props) {
  return lookup(props.something).then(result => {
    return {
      user: result.data,user,
      place: result.data.place,
  });
})

This makes sense from a signature standpoint because the props aren't defined until the promise resolves. However, this is a problem because it means the resolver doesn't know if the props are pre-existing or not.

TBH, reducing duplicate in-flight requests should be handled via your request mechanism. Annoyingly, there are few implementations I found of this:

https://github.com/matthiasak/universal-utils/blob/master/src/fetch.js

Or my own:

export default class HttpActions extends Actions {
  constructor(...args) {
    super(...args);

    this.requests = {};
  }

  get(url, params) {
    const existing = this.cached(arguments);

    if (existing) {
      return existing;
    }

    const request = this.request("get", url, params);

    return this.cache(request, arguments);
  }

  cache(request, signature) {
    const key = JSON.stringify(Array.prototype.slice.call(signature));

    this.requests[key] = request;

    return request;
  }

  cached(signature) {
    const key = JSON.stringify(Array.prototype.slice.call(signature));

    return this.requests[key];
  }
  ...

@ericclemmons
Copy link
Owner

Another backwards-compatible API would be something like:

@resolve(["propA", "propB"], function(props) {
  return promise(something).then((result) => [result.a, result.b]);
})

Because they're positional params, it's basically describing the required response signature should any of the props not exist.

Again, I feel this warrants mentioning...In my own side-projects using Redux (meh), co-locating data requirements via @resolve to their immediate components makes sense & works, but it's difficult to solve the duplicate request issue with Redux & in-flight requests.

If I figure this out, it'll probably solve it for several people.

@nettofarah
Copy link
Author

This all makes sense, @ericclemmons.

re: Caching
It is definitely a solution, but it feels a little brittle. Mostly because it adds cache invalidation to the mix.

re: backwards compatible API
I hacked something together that basically just allows you to use a hash as opposed to an array. But I see the value of taking your approach to it.

I'd love to help out and try to come up with an implementation for this last suggestion 😄

My current solution basically hijacks resolve.js and checks if a complex has been returned from Resolver and then just deconstructs it.

Would resolve.js be the place to make this changes? Or does this sound more like something Resolver should handle?

@MatviiStelmakh
Copy link

MatviiStelmakh commented Nov 7, 2018

Why not do:

@resolve({
  propA, function() { ... },
  propB, function() { ... }
})

small correction : instead of ,
@resolve({
propA: function() { ... },
propB: function() { ... }
})

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

3 participants