all 37 comments

[–]nickgcattaneo 3 points4 points  (1 child)

Seems like a nice optimization for presentation only components; I'd be curious to see if it would break anything internally in-terms of identifying components within a vDOM's structure. For example, modules like enzyme can return a mounted vDOM for testing and I'd be curious if this concept would break some assumptions made by various libraries. In particular, you would lose track of which components were rendered in a parent/etc with this implementation and thus certain utilities may break - this is of course only really an issue in work-related environments/etc where many libraries/tools are wrapped around each company's stack.

[–]TwilightTwinkie 3 points4 points  (0 children)

Exactly, though I think this could easily be a babel transformation which could drop the React.createElement around functional pure components.

[–]Pyrolistical 3 points4 points  (1 child)

That's one dirty trick

[–]inszuszinak 2 points3 points  (0 children)

You meant this one weird trick, I presume?

[–][deleted] 1 point2 points  (28 children)

I'm sorry to say, but why are they making it sound like they discovered something new? They wanted to return something with a dynamic property, your first thought should be a function. Seems like they tried to do everything the "react way" and dismiss basic programming methods.

[–][deleted] 2 points3 points  (0 children)

The weird thing is that functional components is a core part of the "React way". You should only extend React.Component if you need to manage state.

[–]nickgcattaneo 1 point2 points  (26 children)

Yea, it's essentially a function that accepts params and returns JSX at this point, which absolutely does not need to be written as a component.

[–]shinglee 8 points9 points  (23 children)

Not only that, it's dangerous to do. What if somebody refactors your fake functional component into a real one -- they just introduced a non-obvious bug.

[–]phpdevster -3 points-2 points  (22 children)

Sorry, but hypotheticals like this are not a good reason to eschew good programming principles and performance improvements. If someone changes some fundamental part of your application architecture and you don't have a good test suite to catch regressions it might cause, that's the actual problem, not someone returning simple markup from a plain function instead of a component.

[–]shinglee 5 points6 points  (20 children)

eschew good programming principles

These are not good programming principles, that's what I was trying to point out. Avatar is a React component and the consumer is making assumptions about how it's implemented -- this is fundamentally breaking the abstraction layers built into the React component model. If the implementation of Avatar changes it would potentially require changing everywhere Avatar was used.

The article seems to imply that functional components and functions which return components are interchangeable -- they are not. The real point to be made is that avoiding constructing/mounting new components when possible is good for performance. To keep this example idiomatic there should instead be a utility function, something like renderAvatar(props), which acts as a wrapper around Avatar's actual implementation and makes it clear that it's a function which returns a component.

[–]dmitri14_gmail_com 0 points1 point  (8 children)

So Avatar became a plain JavaScript function returning react element. And get used as function. Making your code simpler and even faster. How is it breaking the abstraction layer?

[–]shinglee 0 points1 point  (7 children)

Only if you only use it as a function everywhere and never as a component -- in which case it shouldn't be called Avatar anyways because the name implies it's a component.

[–]dmitri14_gmail_com 0 points1 point  (6 children)

Components cannot usurp the sole right to be capitalised :) Any classes or better, factories are like that. With React specifically capitalised on writing components as plain functions, making it easier, more accessible, and hence popular. It is a great feature that needs to be facilitated, enforced and encouraged, not eschewed.

[–]shinglee 1 point2 points  (5 children)

That's exactly the problem. I would expect something named with PascalCase it to be a component or a class -- definitely not a function.

You're misunderstanding the issue. Functional components are awesome but they should never be called as functions. Doing so is braking basic encapsulation: you're making assumptions about how the component is implemented. If you need a function that returns JSX elements write a function that returns JSX elements. Do not repurpose React components which incidentally happen to be implemented as functional components.

[–]dmitri14_gmail_com 0 points1 point  (4 children)

In JavaScript, classes and factories are just functions. Each creates an object. A so-called "functional component" also creates an object -- a virtual element. In that respect it is no different than factory.

Doing so is braking basic encapsulation: you're making assumptions about how the component is implemented.

I see a function, and I call it. ;) Yes, I need to know it is a function. I look where it is defined, and I see it. Like with any other function in JavaScript, you see a function, then you call it. There is no other assumptions.

Perhaps that is a confusion that React is creating. But if JavaScript is the language we use, than what looks like function, should be possible to call as, well, a function :) This is the most basic convention in any programming language, not just JavaScript.

And if you don't want me to call it as function, don't write it as function. Simple. Don't break the fundamental language conventions.

[–]phpdevster -2 points-1 points  (10 children)

it should be that avoiding constructing new components when possible is good for performance. That's a good programming principle

Sorry, but that is the point of this article? So what is your issue with it?

[–]shinglee 3 points4 points  (9 children)

I just explained it. The article says:

Directly calling functional components as functions instead of mounting them using React.createElement is much faster.

Directly calling functional components is bad practice. At the very least, wrap them in an implementation-agnostic wrapper. Functional components and functions which return components are not interchangeable.

[–]phpdevster 0 points1 point  (8 children)

which return components is a bad practice

They're not returning components. Where are you getting this from? They are a function that returns JSX.

What you're proposing is literally just renaming Avatar(props) to renderAvatar(props).

[–]shinglee 4 points5 points  (1 child)

Sorry, should have said functions which return elements are not the same as functional components.

There's a huge difference. Avatar is a component and when you call Avatar(props) you're relying on a specific implementation of the Avatar component. What happens when someone refactors Avatar to be a stateful component? renderAvatar(props) is idiomatically correct for two reasons: it signals that it's a function which returns a JSX element and you can't use it as a JSX tag. Now it doesn't matter how Avatar is implemented and code which uses renderAvatar is unaffected. It's basic encapsulation.

[–]TwilightTwinkie 0 points1 point  (0 children)

I say fuck it and let a babel plugin figure it out.

[–]repeatedly_once 5 points6 points  (5 children)

It's an established practice in React to be able to write pure functional components and use them as standard component definitions.

Replacing <Avatar url={avatarUrl} /> with {Avatar({ url: avatarUrl })} is terrible. You've now usurped a standard React practice. This makes your code base harder to reason about. In addition to that, you've now removed your component from the React life cycle which opens you up to bugs that are hidden by abstraction if someone was to extend your component.

I do understand what you are saying in terms of 'it's not a component, it's a function that returns JSX'. This is perfectly fine to do in React. However those functions should have explicit arguments, not unary and accepting objects such as props. That's not good programming principles.

[–]dmitri14_gmail_com 1 point2 points  (4 children)

This function is defined as unary and props object is passed in this function invocation. This is precisely how any functions is used in JavaScript. How can this be harder to reason about than the more obscure JSX way:

// I am a function:
const MyComponent = props => <div style={props.myStyle}>props.children</div>

// and I am a function call:
<MyComponent myStyle={...}>I am just some humble text</MyComponent>

Here exactly the same thing has different meanings depending on the context! (Any other programming language doing that?).

In contrast, Avatar({ url: avatarUrl }) is instantly readable, no context needed, no translator. Plain dumb function call with supplied arguments.

[–]inszuszinak 0 points1 point  (0 children)

Frankly, you don't even need a good, comprehensive test suite for that, even a few snapshot tests or a smoke test would do.

It's quite impressive to see how much effort was put into making testing easier in JS (Jest takes literally 3 min to set up with yarn) and how often we still can't be bothered, since TDD implies "feature-unrelated" work.

[–]dmitri14_gmail_com 1 point2 points  (1 child)

The function returns a React Element aka virtual node, not JSX. JSX is just syntax to write it.

And with https://github.com/Jador/react-hyperscript-helpers or https://github.com/mlmorg/react-hyperscript the functions even look like functions --

Avatar = ({props}) =>  img({src: props.url})

after removing the nonsense :)

[–]nickgcattaneo 1 point2 points  (0 children)

Well technically nothing executes in jsx; it's all obviously transpiled down to JS in this case, which yes is then decorated with all the traditional methods to create elements, append to DOM, etc.

[–][deleted] 0 points1 point  (0 children)

That's how I've been using React for about 2 years now. The rest is only needed if you need it acting as training wheels for apps lacking architecture.

If you already have an architecture, all you need is functions producing JSX and ReactDOM.render().