Should forms be separated into a container and a dumb component?

[johanneslumpe]

I'd propose that https://github.com/erikras/react-redux-universal-hot-example/blob/master/src/components/SurveyForm/SurveyForm.js should be split into a container and a dumb form component. The container would connect to the redux-form store and contain the validation and submit functionality. It should only pass its props to the dumb form component. @erikras are you on board with a change like this? If so I could send a PR for that.

[webmasterkai]

I'd love to see that. I think it could be taken a step further and remove the handleSubmit and handleInitialize methods on the Survey component and pass those in as props. It's a little confusing to me what qualifies as a "containers" as what gets placed into the components directory. To me, if it contains jsx that renders into html it should be a component. If this gets approval I would sign up for helping convert other containers/components to stateless functional components where possible.

[johanneslumpe]

@webmasterkai What do you mean with going further? What i meant was that everything should be a prop on the Survey component and it should have its own container component which does the connecting to the store. That way the form component would be a stateless component in react 0.14. Or did I misunderstand what you mean?

[webmasterkai]

Ah perfect. I just wasn't sure based on the link to SurveyForm. I'd love to see Survey and SurveyForm turn into functional components. Willing to help anywhere/way possible.

[johanneslumpe]

Yeah. We would then have Survey + SurveyContainer and SurveyForm + SurverFormContainer.

SurveyContainer would connect to the redux store SurveyFormContainer would use the form decorator

I am not 100% sure whether Survey itself has to really be spread out into a container and a dumb component, because there is not much going on except rendering some static data and then the form itself. The benefit here isn't as big I think. But separating SurveyForm from its container gives us a benefit of better testability (in case the component is decorated more than once for example).

[webmasterkai]

I was thinking one container that wraps Survey. Survey consumes the one handleInitialize property and then does something like <SurveyForm {...rest} />.

[webmasterkai]

Something along the lines of https://github.com/webmasterkai/boiler/blob/master/src/containers/Survey/Survey.js for a container. Untested but it shows the idea. (Please ignore comma-dangle, they are required on the project I'm working on.)

[johanneslumpe]

@webmasterkai yeah that is nice :+1: - I would change Survey and SurveyForm to pure functions though instead of making them classes now. They have no lifecycle hooks or anything, so there is no point to use a class - what do you think?

Also I think the container should export the functions it uses, so they can be tested individually.

[webmasterkai]

Yeah, totally agree 100%. I just didn't get that far in thinking it out.

[johanneslumpe]

Ah ok - but great work so far!

[webmasterkai]

Ok, Survey and SurveyForm are now functions. Now I need to figure out how to move the use of <DocumentMeta /> further up the tree. Having it inside a "dumb" template really bothers me. Maybe just calling it as a function before the return would calm my sensibilities. I think ideally I'd like to see it called within the container if possible...

[kikoanis]

I think <DocumentMeta /> could be defined once in <App /> container to render all default values including title like so

<DocumentMeta {...config.app} />

and then in any container that needs to override any of the document meta data properties to use extend prop i.e.:

<DocumentMeta title={config.app.title + ': Survey'} extend />