didierfranc / react-waterfall

React store built on top of the new context API
MIT License
1.33k stars 77 forks source link

Only re-render if action creator returns an object #99

Open runemadsen opened 5 years ago

runemadsen commented 5 years ago

First of all, thanks for the great library!

Here's a proposal for a feature: To allow an action creator to return false or {} in order not to re-render the connected components.

We're having an issue in our app where we need to have a timeout running every 100ms to fetch updates from a backend. Most of the time, this fetch returns an identical object to that in state and we don't want to re-render all our connected components. So our action creator looks like this (using immutable):

{
  actionsCreators: {
    syncData: async ({ data }) => {
      const newData = await api.get('/data');
      return data.equals(newData) ? {} : { data: newData };
    }
  }
}

So we return an empty object if the data did not change. However, this still causes react-waterfall to re-render every connected component. It seems like it would be easy to implement something where we can return false or an empty object in order not to make react-waterfall re-render.

Is this a PR that you would be interested in?

mstronge commented 5 years ago

@runemadsen Could you utilise the shouldComponentUpdate lifecycle method to solve your issue?

runemadsen commented 5 years ago

Yes, we are currently using shouldComponentUpdate as a workaround. However:

  1. We have hundreds of connected components that have to do deep equality checks every 100ms. This is a significant performance hit that could be entirely eliminated with a single if() statement in react-waterfall that does not break the current API.

  2. I would argue that the current functionality is undesirable. In a component, I can easily fetch new data and only call setState if the data is different. This is currently impossible in react-waterfall, as setState will be called on every action call, even ones that return nothing. This patterns prevents us from really embracing the store philosophy because we have no ability to perform heavy data fetching without also introducing heavy component logic. PureComponent is no help since connect() creates a new object every time it is called, so the shallow equality check does not work.

I will happily send a PR if it's wanted.

bravomartin commented 5 years ago

any interest on this? It would solve my problem too :)

didierfranc commented 5 years ago

@runemadsen A PR will be great and will solve a real issue ;)

vbuch commented 5 years ago

@didierfranc @runemadsen @bravomartin ^ here is a PR

bravomartin commented 5 years ago

Excellent! That's exactly what I need.

Also very clean implementation 👍

@didierfranc please take a look and merge away 💯