Suggestion for with-redux example: using higher-order component to replace repeated codes.

[huzidaha]

The example of next.js with redux has repeated codes in every page, but wouldn't it be better to use higher-order component to replace this kind of repeated codes. For example:

next-connect.js:

import React, { Component, PropTypes } from 'react'
import { Provider, connect } from 'react-redux'
import { createStore, applyMiddleware } from 'redux'
import thunkMiddleware from 'redux-thunk'
import reducer from './reducer'

let store = null

const wrapWithProvider = (PageComponent) => class extends Component {
  static propTypes = {
    initialState: PropTypes.object
  }

  static async getInitialProps (ctx) {
    const { req } = ctx
    const isServer = !!req
    const props = PageComponent.getInitialProps ? await PageComponent.getInitialProps(ctx) : {}
    if (isServer && typeof window === 'undefined') {
      store = createStore(reducer, {}, applyMiddleware(thunkMiddleware))
      props.initialState = store.getState()
    }
    return props
  }

  constructor (props) {
    super(props)
    if (!store) {
      store = createStore(reducer, props.initialState, applyMiddleware(thunkMiddleware))
    }
  }

  render () {
    return (
      <Provider store={store}>
        <PageComponent {...this.props} />
      </Provider>
    )
  }
}

export default (mapStateToProps, mapDispatchToProps) => (PageComponent) => {
  PageComponent = connect(mapStateToProps, mapDispatchToProps)(PageComponent)
  return wrapWithProvider(PageComponent)
}

In every page you can easily use next-connect.js and connect to store like using redux normally:

index.js:

import nextConnect from './next-connect'

class Index extends Component {
  // ...
  render () {...}
}

export nextConnect((state) => state, null)(Index)

You can gain the same abilities like with-redux example does but with convenience.

Just a suggestion, feel free to close.

[huzidaha]

And considering this kind of higer-order component might be commonly used in every next.js project, I wonder if it's a good idea to make it a next.js third-part library?

[katopz]

I think @borellvi and @arunoda try to keep it same as mobx example
Maybe you could try PR as with-redux-thunk instead.

[huzidaha]

@katopz It just changed the code structure of the original version of with-redux example. They're aim for the same purpose.

[katopz]

@huzidaha I know it's easy and made sense and it's a way to go for real life using but in case you introduce another dependency redux-thunk so it should be more clear to new comer if it present separately as with-redux-thunk.

And also FYI : #1196
BTW don't get me wrong I like your example and really hope it get PR in somewhere. :)

[huzidaha]

@katopz Actually thunk thing was copied from the with-redux example . I just wanted to keep it same as original version.

Seems that somebody has made this idea out as a package just a few hours after this issue. Is this just a coincident or has something related to this issue?

[timneutkens]

cc @kirill-konshin

[huzidaha]

I actually want to make a PR. But I am wondering if it's polite to replace his example...

[huzidaha]

I create a PR #1201 which uses the package next-connect-redux I am working on and it's base on the idea above.

See https://github.com/huzidaha/next-connect-redux/blob/master/lib/index.js.

I will finish the docs and tests of next-connect-redux as soon as possible.

[kirill-konshin]

next-connect-redux actually makes exactly the same thing as next-redux-wrapper that was merged yesterday. Same ideas are flowing in the air.

@huzidaha it's pure coincidence, I haven't seen this issue before today, I was working on my stuff for last couple of days, and as you see, I use slightly different approach.

kirill-konshin/react-ssr-playground@0ebca3d here is the link to my playground, dated Feb 16.

https://github.com/huzidaha/next-connect-redux/blob/master/lib/index.js#L15 you have a bug on this line because redux will ignore default state coming from reducer, default state has to be undefined.

Also where is the store memoization, so that on a client there will be just one instance? Have you tested it?

https://github.com/huzidaha/next-connect-redux/blob/master/lib/index.js#L42 connect has 3rd argument mergeProps and 4th argument options which are not supported by your lib.

I suggest to leave the example as is. @huzidaha I can add you as collaborator to my project, if you want.

[huzidaha]

@kirill-konshin I am really sorry for the misunderstanding. And thank you for the suggestions. :P

[rauchg]

@kirill-konshin the PR description referencing the issue made me think his implementation predated yours. I will revert.

[rauchg]

Also, maybe to have the best of both worlds, @kuzihada, you can PR later a little note on the README saying that there's another package available and link to yours ?

[rauchg]

@kirill-konshin apologize for the confusion

[kirill-konshin]

No worries.

[huzidaha]

@rauchg I really appreciate for that.