Problem when conditionally rendering content inside functions

[mairatma]

Say you have some code like this:

function getDiv() {
    return <div>Bottom</div>;
}

function render() {
    return <div>
        {true &&
            <div>
                <div>Top</div>
                {getDiv()}
            </div>
        }
    </div>;
}

IncrementalDOM.patch(element, render);

After patching the element its contents should be:

<div>
  <div>Top</div>
  <div>Bottom</div>
</div>

But they end up being:

<div>
  <div>Bottom</div>
  <div>Top</div>
</div>

Looking at the compiled code, it seems like this is because it's calling getDiv before the right time, causing incremental dom to render it first:

function render() {
  IncrementalDOM.elementOpen('div');
  iDOMHelpers.renderArbitrary(true && iDOMHelpers.jsxWrapper(function (_getDiv) {
    IncrementalDOM.elementOpen('div');
    IncrementalDOM.elementOpen('div');
    IncrementalDOM.text('Top');
    IncrementalDOM.elementClose('div');
    iDOMHelpers.renderArbitrary(_getDiv);
    return IncrementalDOM.elementClose('div');
  }, [getDiv()]));
  return IncrementalDOM.elementClose('div');
}

Removing the {true && ...} part everything works again, so it's something with conditionals + functions.

[jridgewell]

Unfortunately, there's no good way to fix this. Because you've hoisted getDiv outside render, it'll be detected as a "root" rendering function (and it won't add a jsxWrapper).

Possible solutions:

• Turn on fastRoot, which'll try to prioritize the side effects of iDOM over the semantics of JSX
  • Cons: It'll probably break something unrelated in your project.
• Find the referenced function, find out if it returns JSX. If so, put it in a wrapper.
  • Cons:
  • It won't work across modules, only with functions inside the current one.
  • It'll break any side effects you rely on in getDom. In this case, it's a "pure" JSX function, but it'd be pretty simple to write one that isn't.

---

For reference, here's the (snipped) output with fastRoot: true:

function getDiv() {
    elementOpen("div");
    text("Bottom");
    return elementClose("div");
}

function render() {
    elementOpen("div");
    true && (elementOpen("div"), (elementOpen("div"), text("Top"), elementClose("div")), _renderArbitrary(getDiv()), elementClose("div"));
    return elementClose("div");
}

[mairatma]

Would it be possible to transform operations like || and && that are inside jsx elements into their if/else equivalents? It could fix this kind of problem without cons (I think) :)

[jridgewell]

Possibly, as long as there's not a invoked function with JSX inside the conditional expression (madness!).

function check() {
  return <why>do I do this?</why>
}

function render() {
  return <root>
    {true && check() && <tears />}
  </root>;
}

[mairatma]

Agreed, that could never work, it's not a supported use case.

It'd be worth doing this fix for the other cases though, which developers would expect to work.

[mthadley]

So would something like this still be supported?

// Case A
function check(count) {
  return count > 5;
}

function render(data) {
  return <root>
    {true && check(data.count) && <WowSoMany />}
  </root>;
}

Or this:

// Case B
function renderMany(count) {
  return <WowSoMany />;
}

function render(data) {
  return <root>
    {data.count > 5 && renderMany()}
  </root>;
}

[mairatma]

Yeah, I think both these cases you've mentioned already work today, btw, since the jsx function is not inside another element. This change would just make other cases work too.

[mthadley]

Ah ok, cool. The reason I asked is it seemed like Case A above was what @jridgewell was describing:

as long as there's not a invoked function with JSX inside the conditional expression

So modifying Case B, this is what would be fixed, right?

function render(data) {
  return <root>
    {data.count > 5 && <div>{renderMany()}</div>}
  </root>;
}

[mairatma]

Exactly :)