Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates debug logging for SSR #32

Closed
wants to merge 4 commits into from
Closed

Conversation

@aulneau
Copy link
Contributor

@aulneau aulneau commented Jun 29, 2018

This PR addresses some of the logging issues I've had when using redux-bundler with something like next.js. should resolve #24 and #31

  • I've moved all the logging functions to their own doX functions.
  • I also had to add conditionals to the URL bundle selectors/functions as the url bundle wasn't working correctly in a SSR env.
  • I then created a function to do all the logging that I am calling in the debug middleware.

@HenrikJoreteg let me know what you think!

Thanks!
t

@HenrikJoreteg
Copy link
Owner

@HenrikJoreteg HenrikJoreteg commented Jul 6, 2018

Hey thanks @aulneau I'm on vacation, just haven't had time to properly review this yet.

@aulneau
Copy link
Contributor Author

@aulneau aulneau commented Jul 19, 2018

No worries! Enjoy your vacay 👍

@aulneau
Copy link
Contributor Author

@aulneau aulneau commented Aug 3, 2018

Hey @HenrikJorteg just checking in! Do you think you can review this sometime soon? Thanks!

Copy link
Owner

@HenrikJoreteg HenrikJoreteg left a comment

Sorry, the last few weeks have been kind of crazy. Overall, I like it, just the one thing about not wanting to log everything with each action is my main point of feedback. Thanks!

@@ -10,8 +10,7 @@ export default store => next => action => {

if (isDebug) {
console.debug('state:', store.getState())
self.logSelectors && self.logSelectors()
self.logNextReaction && self.logNextReaction()
store.doLogEverything && store.doLogEverything()
Copy link
Owner

@HenrikJoreteg HenrikJoreteg Aug 3, 2018

I like the idea of the doLogEverything but, this switch means that it logs all installed bundles and all action creators after each action, which is just a lot of noise. Especially since those items don't change each time. I think it makes more sense to just do the selectors / next reaction here still.

Copy link
Owner

@HenrikJoreteg HenrikJoreteg Aug 3, 2018

screen shot 2018-08-03 at 7 40 00 am

doLogBundles: () => ({ store }) => {
const names = store.meta.chunks[0].bundleNames
return console.log(
'%cinstalled bundles:\n %c%s',
Copy link
Owner

@HenrikJoreteg HenrikJoreteg Aug 3, 2018

I know this isn't new, so feel free to leave it alone, but since you're in here anyway, feel free to look at why the display of the bundle names is so odd in the browser:

screen shot 2018-08-03 at 7 41 17 am

@HenrikJoreteg
Copy link
Owner

@HenrikJoreteg HenrikJoreteg commented Jun 7, 2019

I ended up re-working the debug bundle, it is not created/configurable programmatically and works in node.js (https://github.com/HenrikJoreteg/redux-bundler/blob/master/docs/misc/changelog.md#change-log).

Thanks for your work on this, i took some of the ideas here (and gave you credit in the change log).

Cheers!

@aulneau
Copy link
Contributor Author

@aulneau aulneau commented Jun 7, 2019

@HenrikJoreteg Thank you! :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants