Javascript: [React] [Question] Naming convention for methods that return jsx partials

[spen]

Hi All!

I'm not sure this is actually something worth adding to the guidelines so I thought I'd get feedback/ opinions before opening a PR myself...

My suggestion is that methods that render jsx partials be prefixed with 'render'

For instance:

// bad - suggests data getting
getHeading = () => {
    return <Header>{this.props.header}</Header>
}

// good - returns JSX to be rendered
renderHeading = () => {
    return <Header>{this.props.header}</Header>
}

Of course the method doesn't actually render any JSX in to the DOM at this stage so it could be argued that render___ is misleading.

Any thoughts on this?
If it seems reasonable I can put together a more formal PR this evening - Thanks!

[ljharb]

In fact, there shouldn't be any renderFoo methods/functions at all - in this case, that should just be an SFC, or better, just rendering the <Header> inline.

I'm not sure such a naming convention is particularly useful given that JSX-returning functions almost always should be SFCs, or inline callbacks, instead.

[spen]

I agree on the SFCs actually, I just see lots of these methods around!
On second thought, having something in the guidelines that recommended a naming convention for something that shouldn't be done in the first place could actually encourage people not to use SFCs - which wouldn't be so good!
Good points @ljharb - Thanks!

[Bamieh]

@spentay why it shouldnt be done? i see this case in almost every open source react component at airbnb! plus it makes the code a little neater. i see things like maybeRenderFoo and renderBar all over, i cant pinpoint why its a bad convention, its definitely not a replacement for SFC's.

[ljharb]

@Bamieh Please link me to where you see this? I'll update them all to use SFCs ASAP.

It's partly a bad convention because it's harder to test, and if the functionality needs to be extracted to a function, it should be properly tested.

[Bamieh]

@ljharb Thanks for the clarification. In the dateRangePicker.jsx in react-dates repo, there are 2-3 functions like that, like maybeRenderDayPickerWithPortal and renderDayPicker. But i believe this whole component should be refactored to have the portal in another component/wrapper, since its not the calendar's concern where its at. Its up to the user to use a portal or not.

https://github.com/airbnb/react-dates/blob/master/src/components/DateRangePicker.jsx#L317

[spen]

Thanks for the conversation @Bamieh & @ljharb - learning lots!
I'd be more than happy to open some PR's and help out!

I'm personally not a big fan of the maybeRender__ pattern anyway, it feels ambiguous!
I'm usually in preference to something like: { isSomething && <Component /> } or a ternary

[ljharb]

@Bamieh thanks, would you mind filing an issue on react-dates for this?

[Bamieh]

@ljharb sure, i will tomorrow morning ;)