search

ctrlplusb / react-universally

A starter kit for universal react applications.
MIT License
1.7k stars 244 forks source link

Adding logging for redux actions, and updating DevTools syntax #517

Closed oyeanuj closed 6 years ago

oyeanuj commented 6 years ago

This PR improves the dev experience of working with redux by:

  1. Adding logging of Redux actions (along with next and previous state) in dev mode, on both browser and node CLI.

  2. Adds redux-unhandled-actions, a tiny middleware that helps you detect (only in dev mode) when your actions do not create a new state object.

Alongside, it also updates the Redux DevTools syntax to the latest recommend one.

Hope it helps!

oyeanuj commented 6 years ago

One thing I'd need someone to look at is that I had to define global variables using DefinePlugin in Webpack since process.env.BUILD_FLAG were not reliably representing the environment that the code was running in. Maybe I was missing something since they feel quite similar to each other..

SleepWalker commented 6 years ago

and why not to use https://github.com/zalmoxisus/redux-devtools-extension? It should solve the first issue

Regarding the second one, I'd say, that it would be better to handle this using unit tests and static code analyzing.

oyeanuj commented 6 years ago

@SleepWalker comments below:

and why not to use zalmoxisus/redux-devtools-extension? It should solve the first issue

This is what we are using in the project, just utilizing the chrome extension rather than another package. That is prior to this PR, and this PR does not change that rather just updates the syntax.

Regarding the second one, I'd say, that it would be better to handle this using unit tests and static code analyzing.

I believe this middleware is a really effective, low-cost best practice whereas unit tests and static code analyzing involve more effort and opinion.

SleepWalker commented 6 years ago

@oyeanuj I mean, that redux-logger does the same thing, that redux-devtools-extension. Instead of using redux-cli-logger, better to run app with SSR: false or with debugger attached. Devtools extension has 'remote' mode too (I have not tested it, however).

I believe this middleware is a really effective

Detecting bugs at runtime is not really effective. I think this middleware seems to be out of "opinionated" scope. If someone will need such middleware, he can add it in his project, but this is not required for every user.

oyeanuj commented 6 years ago

@SleepWalker Thanks for the comments! I can see your points, but below is where I differ slightly on them.

that redux-logger does the same thing, that redux-devtools-extension.

Over time, it has been more convenient for me to see those actions in conjunction with other logging statements as I debug. It prevents an extra tab-switching. Not that a big deal, but over time has saved me a lot of tediousness.

Instead of using redux-cli-logger, better to run app with SSR: false or with debugger attached. Devtools extension has 'remote' mode too (I have not tested it, however).

I think ssr: false isn't an ideal solution since it is not replicating the experience accurately. The CLI logger also helps debugging when running tests. That said, the Devtools extension's 'remote mode' might be a better option if we can add that. I just haven't gotten it to work when I tried it last.

All said and done, I'm okay with not adding these, if that's the consensus. I offered to add these since I always use them as a best practice.