Remove createFactory calls

[nchevobbe]

I wonder if we could get rid of all the createFactory calls we have in our codebase.

From the React doc :

This helper is considered legacy, and we encourage you to either use JSX or use React.createElement() directly instead.

Even if the createFactory function does little, it still creates a bound function that stays around when the app runs.

I expect the overhead to be quite small, but removing something that is deprecated seems to be a good move, especially with Mike's work on evolving the codebase to be Fiber compliant.

Next question is style, since it would turn something like :

const A = createFactory(require("A"))
[…]
render() {
  return A(props);
}

into

const A = require("A")
[…]
render() {
  return React.createElement(A, props);
}

it seems okay for one-level component, but if you have some deep tree, this can be weird

render() {
  return React.createElement(A, {
     …
    }, 
    React.createElement(B, {
         …
      },
      React.createElement(C, {
         …
      },
  )));
}

This can be mitigated with a shorthand:

const ce = React.createElement;
render() {
  return ce(A, {
     …
    }, 
    ce(B, {
         …
      },
      ce(C, {
         …
      },
  )));
}

but still look less readable (IMO) than :

render() {
  return A({
     …
    }, 
    B({
         …
      },
      C({
         …
      },
  )));
}

but it may just be a matter of habit.

What's you're take on this, should we do it ?

[julienw]

+1 note that React's doc proposes to use e as alias to createElement to make it even shorter.

[gregtatum]

I would disagree on this. I don't want to clutter up our already very verbose render calls with more clutter, especially since we can't use JSX. The memory cost of the bound functions is trivial. If we don't use the actual deprecated function, I would still prefer having some kind of custom factory function.

Now if you propose moving to JSX... 🤓

[jryans]

At the moment, I feel like we should not do this, based purely on style, since it seems to make things more verbose for no clear gain.

While createFactory is labeled "legacy", I couldn't find any clear signal that they actually plan to remove it. There is the following comment about it in React source:

Legacy hook TODO: Warn if this is accessed

which was added 3 years ago (2014), but it appears nothing more has happened since then.

If the situation changes such that using createFactory blocks a React upgrade or a fancy feature we want to switch to, then it seems worth considering this again at that time.

[juliandescottes]

Looks like there are constructive arguments to keep the current approach. It is true that our non JSX React code is already quite hard to read, so adding more visual clutter seems bad. I also could not find any reference pointing at createFactory being removed in the near future.

@nchevobbe do you think we can review this RFC or is more time needed here?

[nchevobbe]

I think we can review it (and I think greg and jryans made good points against it :) )

[juliandescottes]

This RFC was reviewed and rejected: the createFactory API is not planned for removal in React and removing it would make our code harder to read.