Update templates and generators

[outdooricon]

It is pretty confusing when you’re presented with text stating that these default container implementations aren’t SFC because they need to hot reload, but PureComponents doesn’t hot reload. If we've taken the step to bring them to PureComponent, then it feels like why not just take the final step and bring them to SFC.

Also, the spacing in the generated code didn't quite match the work done in #1275, so I added some to bring them in-line.

[coveralls]

Coverage remained the same at 98.864% when pulling 1e0f1c0 on outdooricon:generatorTemplateCleanup into 68abb90 on mxstbr:3.4.

[gihrig]

I suppose there is reason to suggest that SFC should be the default choice, as that is (should be) the most frequently generated component type.

What I would really like to see is to make HMR work with class-based components.

[outdooricon]

@gihrig I agree, that's what I would want too. I've been following #1068 and have had hope that it would get somewhere. But based on https://github.com/danmartinez101/babel-preset-react-hmre/issues/46 and especially @gaearon just outright stepping away from RHL, that it's not headed the direction we need. It would seem better to be exposing preferred convention in the clean base. Especially since PureComponent functions differently than the component comments indicate.

This is all of course, imho, and perhaps @mxstbr would like to kill this PR right now :)

[mxstbr]

The issue is that we get tons of issues "Hot reloading isn't working!!!1111!!!!" if we do this. (that's what happened initially, until we changed it)

I appreciate you taking the time to submit this PR, but sadly the class -> SFC conversion we don't really want to merge due to the above. 😕 I'd appreciate a cherry-picked PR with the other fixes?

[outdooricon]

@mxstbr I think what I'm trying to get at is hot reloading doesn't work for PureComponent either, so why not go to SFC

[mxstbr]

...oh yeah, that's a good point. I didn't realise we switched that to PureComponent... Shall we update it to class instead?

[outdooricon]

@mxstbr would you mind reopening this so we don't loose track of it?

We could go back to Class, but based on danmartinez101/babel-preset-react-hmre#46 and especially @gaearon just outright stepping away from RHL, would it be worth considering a move away from it towards the better code instead?

[mihir0x69]

@mxstbr would you mind reopening this so we don't loose track of it?

Reopening this if you want to do something with it.

[gaearon]

To be clear https://github.com/danmartinez101/babel-preset-react-hmre has nothing to do with RHL (it’s a completely different approach). If RHL doesn’t work with pure components, raise an issue in RHL repo. There’s nothing that I’m aware of that would make them different.

[outdooricon]

@gaearon Thanks for the clarification. I more just brought it up to show that we don't seem to have many options anymore, since babel-preset-react-hmre is no longer maintained and there doesn't seem to be much hope for RHL based on your comment about your involvement in it.

[gaearon]

Just because I don't actively participate in a project and am busy with other things doesn't mean there's not much hope for it IMO.

[outdooricon]

@gaearon That's actually comforting to hear. I think you carry a lot of weight within the community, so when you back away from something, it's easy to read that as something we should all be backing away from.

@mxstbr Does this revive the drive for switching to RHL v3 then? It seems like that work kind of died.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.