Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proposal] Alternative 'React.createElement' api and JSX output #4920

Closed
osi-oswald opened this issue Sep 20, 2015 · 6 comments
Closed

[Proposal] Alternative 'React.createElement' api and JSX output #4920

osi-oswald opened this issue Sep 20, 2015 · 6 comments

Comments

@osi-oswald
Copy link

Problem

The current React.createElement api is quite limiting in regards of control flow statements.

Consider the following example

if (condition) {
  <div>hello world</div>
}

will happily be transformed into

if (condition) {
  React.createElement("div", null, "hello world")
}

but as soon as I try to wrap the very same JSX statement in a component (ex. a 'div') it blows up

<div>
  {if (condition) {
    <div>hello world</div>
  }}
</div>

It would be transformed into invalid JS because of how React.createElement is set up

React.createElement("div", null, 
  if (condition) {
    React.createElement("div", null, "hello world")
  }
)

Workarounds

Of course you can workaround this issue by using the ternary expression (only usable for trivial 'if bodies')

<div>
  {condition ? <div>hello</div> : null}
</div>

or a temporary variable (if the 'if body' is more complicated)

var conditionResult;
if (condition) {
  conditionResult = <div>hello</div>
}
<div>
  {conditionResult}
</div>

This 'splitting' of html hurts the readability quite a lot in my eyes. And worse, I am forced to do this 'splitting', there is no other choice :-(

...at least until now :-)

Proposed Solution

Change (or overload) the React.createElement signature from a list of childrens to a single append function

React.createElement(type, props, ...children) // current signature
React.createElement(type, props, appendFn) // proposed signature

How does this 'appendFn' approach work? Let's explain it through examples:

// simple JSX
<div id="msg">
  Hello
  <b>World!</b>
</div>

// current JS output
React.createElement("div", {id: "msg"}, 
  "Hello", 
  React.createElement("b", null, "World!")
)

// proposed JS output (using the new signature)
React.createElement("div", {id: "msg"}, append => {
  append("Hello");
  append(React.createElement("b", null, "World!"));
})

So 'append' does nothing more than what happens with the '...children' in the old signature anyway. It appends them to the component. The big difference is, we are now in a function body instead of the limiting parameter list => big win!

Some more examples with control flow statements

// 'if' example
<div id="msg">
  Hello
  {if (condition) {
    <b>World!</b>
  }}
</div>

// current JS output is not valid :-(
React.createElement("div", {id: "msg"}, 
  "Hello", 
  if (condition) {
    React.createElement("b", null, "World!")
  }
)

// proposed JS output is valid :-)
React.createElement("div", {id: "msg"}, append => {
  append("Hello");
  if (condition) {
    append(React.createElement("b", null, "World!"));
  }
})
// 'switch' example
<div id="msg">
  Hello
  {switch (value) {
    case 'foo': <b>Foooo!</b>; break;
    case 'bar': <b>Baaar!</b>; break;
    default: <b>World!</b>
  }}
</div>

// current JS output is not valid :-(

// proposed JS output
React.createElement("div", {id: "msg"}, append => {
  append("Hello");
  switch (value) {
    case 'foo': append(React.createElement("b", null, "Foooo!")); break;
    case 'bar': append(React.createElement("b", null, "Baaar!")); break;
    default: append(React.createElement("b", null, "World!"))
  }
})
// 'for .. of' example
<div id="msg">
  Hello
  {for (var person of persons) {
    <b>{person.name}, </b>
  }}
</div>

// current JS output is not valid :-(

// proposed JS output
// (now you can use native JS loops instead of depending on Array.map)
React.createElement("div", {id: "msg"}, append => {
  append("Hello");
  for (var person of persons) {
    append(React.createElement("b", null, person.name, ", "))
  }
})

Background

My roots as web developer lie in the Razor Template Syntax which allows to mix C# with HTML much like JSX does with JS and HTML. That's why I felt a strong connection towards React right from the first day, because mixing a programming language with HTML already felt so natural. Now the only gap left to feel as flexible as back in the old days (with Razor) is this very issue I try to address here in this post. I hope you consider my approach and make React even more awesome than it already is! :D

@osi-oswald
Copy link
Author

A bit more complex example taken from the React TodoMVC

With current syntax (forced to 'split up')

var footer;
var main;
var todos = this.props.model.todos;

var shownTodos = todos.filter(function (todo) {
  switch (this.state.nowShowing) {
  case app.ACTIVE_TODOS:
    return !todo.completed;
  case app.COMPLETED_TODOS:
    return todo.completed;
  default:
    return true;
  }
}, this);

var todoItems = shownTodos.map(function (todo) {
  return (
    <TodoItem
      key={todo.id}
      todo={todo}
      onToggle={this.toggle.bind(this, todo)}
      onDestroy={this.destroy.bind(this, todo)}
      onEdit={this.edit.bind(this, todo)}
      editing={this.state.editing === todo.id}
      onSave={this.save.bind(this, todo)}
      onCancel={this.cancel}
    />
  );
}, this);

var activeTodoCount = todos.reduce(function (accum, todo) {
  return todo.completed ? accum : accum + 1;
}, 0);

var completedCount = todos.length - activeTodoCount;

if (activeTodoCount || completedCount) {
  footer =
    <TodoFooter
      count={activeTodoCount}
      completedCount={completedCount}
      nowShowing={this.state.nowShowing}
      onClearCompleted={this.clearCompleted}
    />;
}

if (todos.length) {
  main = (
    <section className="main">
      <input
        className="toggle-all"
        type="checkbox"
        onChange={this.toggleAll}
        checked={activeTodoCount === 0}
      />
      <ul className="todo-list">
        {todoItems}
      </ul>
    </section>
  );
}

return (
  <div>
    <header className="header">
      <h1>todos</h1>
      <input
        ref="newField"
        className="new-todo"
        placeholder="What needs to be done?"
        onKeyDown={this.handleNewTodoKeyDown}
        autoFocus={true}
      />
    </header>
    {main}
    {footer}
  </div>
);

With the proposed syntax ('split up' by choice)

var todos = this.props.model.todos;

var shownTodos = todos.filter(function (todo) {
  switch (this.state.nowShowing) {
  case app.ACTIVE_TODOS:
    return !todo.completed;
  case app.COMPLETED_TODOS:
    return todo.completed;
  default:
    return true;
  }
}, this);

var activeTodoCount = todos.reduce(function (accum, todo) {
  return todo.completed ? accum : accum + 1;
}, 0);

var completedCount = todos.length - activeTodoCount;

return (
  <div>
    <header className="header">
      <h1>todos</h1>
      <input
        ref="newField"
        className="new-todo"
        placeholder="What needs to be done?"
        onKeyDown={this.handleNewTodoKeyDown}
        autoFocus={true}
      />
    </header>
    {if (todos.length) {
      <section className="main">
        <input
          className="toggle-all"
          type="checkbox"
          onChange={this.toggleAll}
          checked={activeTodoCount === 0}
        />
        <ul className="todo-list">
          {for (todo of shownTodos) {
            <TodoItem
              key={todo.id}
              todo={todo}
              onToggle={this.toggle.bind(this, todo)}
              onDestroy={this.destroy.bind(this, todo)}
              onEdit={this.edit.bind(this, todo)}
              editing={this.state.editing === todo.id}
              onSave={this.save.bind(this, todo)}
              onCancel={this.cancel}
            />
          }}
        </ul>
      </section>
    }}
    {if (activeTodoCount || completedCount) {
      <TodoFooter
        count={activeTodoCount}
        completedCount={completedCount}
        nowShowing={this.state.nowShowing}
        onClearCompleted={this.clearCompleted}
      />
    }}
  </div>
);

Which one do you like more? Of course you can debate about that. For me the first one feels like reading backwards (have to look things up) and the second one feels more natural to read (in one flow). But in the end it should be the decision of the developer which approach he finds more appealing, and not be forced to use either way (like he is now).

@syranide
Copy link
Contributor

You can accomplish what you want without extending the core API. It's also important to note that this API design is a deceptively bad idea in React due to how implicit keys work, using an append-style API largely requires you to provide explicit keys and failure to do so will cause unintended remounts (which is really really bad).

The following already works today:

<div>
  {() => {
    if (condition) {
      return <div>hello</div>;
    }
  }()}
</div>
<div>
  {do {
    if (condition) {
      return <div>hello</div>;
    }
  }}
</div>

The syntax isn't nice but it illustrates a better way of dealing with these problems that doesn't introduce any problematic side-effects. The syntax is a separate problem and there have been some talks about switching {} to be implicit do-expressions (I don't know where the discussions are now on this though).

@osi-oswald
Copy link
Author

(I did not know about the 'do-syntax', how do I get it to work in the JSX Compiler?)

I see that my proposed append-style API would mess with implicit keys, which would be really bad. I therefore take it that my solution will not gonna work ;-)

But the need to write something natural like

<div id="msg">
  Hello
  {if (condition) {
    <b>World!</b>
  }}
</div>

is still at stake. (Please note the non-existent return statement inside the 'if-body')

As a developer it feels strange/inconsistent that something (ex. the 'if-statement') works in the root scope but not anymore when nested.

@syranide
Copy link
Contributor

(I did not know about the 'do-syntax', how do I get it to work in the JSX Compiler?)

JSX compiler is deprecated in favor of babeljs, IIRC babeljs supports that feature.

As a developer it feels strange/inconsistent that something (ex. the 'if-statement') works in the root scope but not anymore when nested.

Not really IMHO, just as you can't put an if-statement inside an expression in JS (e.g. inside a function call), you can't put an if-statement inside the expression of a JSX element. But I would personally agree this is inconvenient at times, but that's not necessarily a problem JSX can or should fix (everything comes at a cost)... but there are discussions:

reactjs/react-future#35 facebook/jsx#39

@osi-oswald
Copy link
Author

Thanks for the info, happy to see discussions about it. And I can see do-expressions have even an implicit 'return', nice. (It almost looks like implicit do-expressions would do the job.)

As far as I'm concerned, you can close this issue.

@jimfb
Copy link
Contributor

jimfb commented Sep 21, 2015

I'm going to close this out, in favor of tracking it via the issues @syranide linked. Thanks.

@jimfb jimfb closed this as completed Sep 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants