zalmoxisus / crossbuilder

Building web, Electron, Cordova and Chrome apps, and cross-browser extensions with React, Redux and Webpack. "Write once, deploy everywhere" concept in practice.
MIT License
484 stars 50 forks source link

Using React components for Chrome badges? #29

Closed unmanbearpig closed 8 years ago

unmanbearpig commented 8 years ago

Hi!

I'm new to React and just started learning Redux so I might be completely wrong, and I'm not sure if it's going to work, please let me know what you think.

My understanding how badges currently work

I've looked at the code and to me it looks like you're using redux-notify to send NOTIFY_SEND and NOTIFY_RECEIVE events that you then catch in a reducer and set the badge: https://github.com/zalmoxisus/browser-redux/blob/master/src/app/reducers/counter.js#L6

  if (
    typeof window === 'object' && window.bgBadge &&
    (action.type === NOTIFY_SEND || action.type === NOTIFY_RECEIVE)
  ) {
    window.bgBadge(state.count); return state;
  }

The Redux docs say that reducers should be pure and not have any side effects: http://redux.js.org/docs/basics/Reducers.html

For now, just remember that the reducer must be pure. Given the same arguments, it should calculate the next state and return it. No surprises. No side effects. No API calls. No mutations. Just a calculation.

Also, I think it's kind of a lot of new concepts for a relatively small feature.

Possible alternative

I'm thinking that wouldn't it be simpler to have a React component on the background page that would take count from the store as a prop and instead of rendering HTML, set the badge?

Don't you think it would be a better implementation? Do you see any issues with my approach?

I'm just learning Redux, so any feedback would be very helpful and appreciated. Thanks!

zalmoxisus commented 8 years ago

Hey,

We used to just have a store.subscribe which is the best way to deal with side effects, and could be a good solution for most of cases. Adding a react component connected with redux-react will do the same, but we don't render anything. The problems come when you have a really big store with a lot of action dispatched, then having this state traversal will slow down the performance as we don't know from where what action was dispatched. More details in #18.

Another Redux way would be to use this widget in actions instead of reducers. The inconvenience there is that we have to call it for every action that changes counter. In our case it is only increment and decrement, but in a real application there could be a lot of actions. Another problem is that it wouldn't work with Redux Devtools.

The third solution would be to use a store enhancer. I'll try to explore that, but I'm not sure that it will work with Redux DevTools as we need the badge to be changed when toggling or redoing actions from there.

The recommendation about not having API calls in reducers is connected to the fact that while debugging (with Redux DevTools) we redo / toggle actions, and having an ajax call, for example, will cause adding or changing data in the database everytime. But here we actually need it.

unmanbearpig commented 8 years ago

Thanks for a thorough explanation, @zalmoxisus. store.subscribe makes a lot more sense than React components. It seems to me it's the most conventional approach, so I'm gonna go with it, and deal with performance issues when and if they come up.