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

Pattern for sequential dependent requests #73

Closed
RohovDmytro opened this issue May 2, 2017 · 6 comments
Closed

Pattern for sequential dependent requests #73

RohovDmytro opened this issue May 2, 2017 · 6 comments

Comments

@RohovDmytro
Copy link
Contributor

The case

  • Request B need data that it will get only from request A.

Done 😸

Solution (so-so one)

It would be ideal to see all requests for a component in one place — in connectRequest hoc.

What I came up is some idea like this:

// ...
connectRequest(props => {
  const configs = [];
  configs.push(getQueryConfigA());
  if(props.dataForRequestB){
    configs.push(getQueryConfigB(props.dataForRequestB));
  }
  return configs;
}})
// ...

What I don't like

While it's seems quite nice, but we pollute our prop property with data not specific for views. Best I can do now is to use another HOC to remove redux-query specific props later in composition.

?

Your thoughts?

@RohovDmytro RohovDmytro changed the title [discussion] Pattern for fire dependent requests [discussion] Pattern for sequential requests May 2, 2017
@RohovDmytro RohovDmytro changed the title [discussion] Pattern for sequential requests [discussion] Pattern for sequential dependant requests May 2, 2017
@RohovDmytro RohovDmytro changed the title [discussion] Pattern for sequential dependant requests [discussion] Pattern for sequential dependent requests May 2, 2017
@RohovDmytro RohovDmytro reopened this May 11, 2017
@ryanashcraft
Copy link
Contributor

@rogovdm Hmm. What is the problem you're trying to avoid by not having these props passed down to your component?

@RohovDmytro
Copy link
Contributor Author

What is the problem you're trying to avoid by not having these props passed down to your component?

  • Performance. Less props - less rerendering. We can omit those props manually, but it feels hacky.
  • Visual polluting of dev tools with unnecessary props. Like, they don't have to be there, but they are. They do not belong to visual representations. but still there.

@ryanashcraft
Copy link
Contributor

Yeah that's what I figured. Makes sense. What do you think about a new option for connectRequest - something like omitProps: Array<string> to strip certain props before passing to the wrapped component?

@RohovDmytro
Copy link
Contributor Author

RohovDmytro commented May 14, 2017

I am writing my app which uses redux-query and I am coming up with my patterns for fetching data. I feel like it will evolve even more, so will write up later.

Where my mind goes

For me I feel like connectRequest should also have... mapStateToProps parameter. Because... why not? 😺 What I am basically doing in my current react-redux mapStateToProps functions is selecting part of the data from state just to pass it to connectRequest. Yeah, some data I do need for an underlying component, but some of it needed just for requests. Separate mapStateToProps to the rescue.

Duplicating selectors

There will be shared data between selected data for a component and selected data for requests. I believe there will be shared pool of data in most cases. But I am okay with that duplication in favor of separating of concerns.

We can always put shared data into mapStateToData function and use it in mapStateToProps (for connect) and in mapStateToProps (for connectRequest). Composition! 😺

And memoization will solve the rest of the problems.

This is what I am thinking about. Mm?

@ryanashcraft
Copy link
Contributor

@rogovdm I see where you're coming from and the use cases. What would be the new signature for connectRequest? mapStateToData would be the second parameter?

Other alternatives:

  1. If mapPropsToConfigs returns a function, the function is called with the state and its return value would be interpreted as the configs instead. Sorta like mapStateToProps allows.
  2. Pass state as the second parameter to mapPropsToConfigs . Although this feels gross because it's the exact reverse of mapStateToProps.

I'm personally leaning towards option 1.

@ryanashcraft ryanashcraft changed the title [discussion] Pattern for sequential dependent requests Pattern for sequential dependent requests Oct 14, 2017
@ryanashcraft
Copy link
Contributor

I am deciding to close this issue. I am resistant to investing any more into the connectRequest HOC by making that API more complicated. I do think that this won't be as much of an issue if you start using our new hooks API when that becomes available (see #129).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants