brocoders / redux-async-connect

It allows you to request async data, store them in redux state and connect them to your react component.
645 stars 103 forks source link

Example with real reducers #9

Open Keats opened 8 years ago

Keats commented 8 years ago

Hey,

I've just seen this pr https://github.com/erikras/react-redux-universal-hot-example/pull/833 which looks exactly like what I need to do in my own app as well (redux-router -> redux-simple-router + changing from middleware to load props to something like redux-async-connect).

The example in the README is for a very basic promise without dispatch so here are my thoughts:

sars commented 8 years ago

@Keats , sorry for lack of docs, going to make it shortly

what is helper? looking at the PR above i see that i can put basically anything in it

yes, you can put there anything you want. the store is there by default.

in the AppComponent, the second param is store and not helpers

yes, static reduxAsyncConnect method accepts 3 params: params, store, helpers I did not use decorators in that PR, because it requires redo much more code, and diff will be too big there.

Keats commented 8 years ago

No worries take your time it's a new project. I'll probably try redux-async-connect today or tomorrow so maybe i'll do a PR for the docs at that point :)

sars commented 8 years ago

@Keats , it would be great :)

Keats commented 8 years ago

Using this issue for an unrelated thing, just switched it and it works great. One problem though: is there a way to not reload data on query change in the url? Ie i have a dashboard and filtering updates the search param in the url but the data is already there so i don't want to re-fetch it on each filter. Adding an option to only fetch on location chance could be nice if possible

sars commented 8 years ago

@Keats I added docs: https://github.com/Rezonans/redux-async-connect/blob/master/docs/API.MD

About your question. It's difficult to recognise if we need reload or not automatically - everybody has their own cases here...

I would recommend to return undefined instead of promise when you don't want to reload data.

Keats commented 8 years ago

Thanks for the docs! Could we get the location from the router than just the params?

An alternative for avoiding refetching would be to add a props shouldFetch to ReduxAsyncConnect that looks like (location, store) => bool that defaults to () => true but would allow people to define their own fetching policy, if that makes sense

sars commented 8 years ago

@Keats , you can pass any helpers you want... route location in particular.

About shouldFetch - interesting idea.... looks like make sense

Keats commented 8 years ago

Just tried, the issue is that you don't get the previous location with redux-simple-router so you have nothing to compare. Ideally you would have something like shouldComponentFetch(prevLocation, nextLocation)

Ended up writing a helper:

let LAST_LOCATION = null;
function onlySearchChanged(location) {
  if (LAST_LOCATION === null || location === null) {
    LAST_LOCATION = location;
    return false;
  }
  const pathnameChanged = location.pathname === LAST_LOCATION.pathname;
  LAST_LOCATION = location;
  return pathnameChanged;
}

A bit ugly but does the job.

sars commented 8 years ago

hm.... i think it can depends on router params also, it can even depends on current time ...

Keats commented 8 years ago

Yeah this was for my particular usecase

sars commented 8 years ago

@Keats https://github.com/Rezonans/redux-async-connect/blob/master/docs/API.MD#asyncconnect-decorator You can return undefined so far.... does it cover your case?

Keats commented 8 years ago

Yep i return undefined depending on what the method above returns in my static fn