ericclemmons / react-resolver

Async rendering & data-fetching for universal React applications.
https://ericclemmons.github.io/react-resolver
Other
1.65k stars 52 forks source link

Allow factory to decide which props to re-use? #119

Closed salmanm closed 7 years ago

salmanm commented 8 years ago

As per Resolver.js#L151, it ignores existing props.

IMO resolver should not be smart in this regard. It should be upto the developer to decide if he wants to refetch the data based on some condition or just use existing data.

I'm using Redux, and my use case is the following.

@connect(({ users }) => { users: users.data })
@resolve('users', () => callApi('/users'))
export default class ...

The resolve factory is never called because of hasOwnProperty check. Currently, I'm doing a simple workaround by prepending an _ to the prop, which seems a bit dirty solution to me.

Since it already passes props to the factory, I would expect this check to be removed and let the factory decide if it wants to return the same data, or fetch it.

Thoughts?

monder commented 8 years ago

I'm not sure exactly if I understood the use-case. react-resolver just resolves property if the component does not have it already,- which seems logical to me. If the property is defined already, it should be valid and up to date, if not - let the whole component re-render and fetch the data.

In your example, users property comes from the redux store, and you want for react-resolver to fetch the data if there is none. But how do you plan to set the data back to the store? I'm not sure that mixing redux and react-resolver is needed here as it could be done just by using either one or the other.

By "prepending an _ to the prop" you mean in @connect? Like:

@connect(({ users }) => { _users: users.data })
@resolve('users', ({ _users }) => _users ? _users : callApi('/users'))
export default class ...
salmanm commented 8 years ago

I'm using Redux as well as Resolver both. I put it in the Redux store on componentDidMount (at which point I already have the data loaded using Resolver). For subsequent requests I can read from Redux and skip the XHR.

Now, I am not sure if I should be decorating my component with connect first and then resolver, or the other way around (Let me know your opinion).

react-resolver just resolves property if the component does not have it already

But it checks it by hasOwnProperty, so even when the data is undefined in Redux store, the props will still haveOwnProperty, and the check will think the property already exist.

I am not against checking if the prop already exist, but I think it should be done within the factory. If you have a reason not to do so, then at lease we should check prop's availability by checking it to be non-undefined.

Makes sense?

ericclemmons commented 7 years ago

The scenario makes sense to me, but, when using Redux (or any other application-state manager), React Resolver may not fit in well with it.

I believe redial is the goto for this scenario.

Reason being, react-resolver was never intended to have to worry about the application changing (and re-re-resolving) unless the route/page changed.

I've personally done the "fake prop so I can dynamically change the real prop value" technique @monder has illustrated above, but I don't know that it's bringing clarity to your project vs. doing things the "redux way".

I hope this helps!

monder commented 7 years ago

Published v3.1.0 on npm. I suppose this issue could be closed as well.

salmanm commented 7 years ago

Excellent! Thanks