Allow invalid render return values in the React reconciler

[trueadm]

Release notes: none

This unblocks an issue found in #2494.

After looking into #2494, which showed that without that PR, some conditions with && weren't being passed correctly to the React reconciler to be properly inlined. With this PR applied however, this issue no longer occurs but we do run into another problem.

The React reconciler will attempt to evaluate, resolve and inline all paths. Typically, if a "value" that a React component returned wasn't that of a valid type (string, ReactElement, null and some other objects) we would throw an ExpectedBailOut. This turned out to break logic in common use-case though: && conditions.

Take the below example though:

function Child() {
  return <div>This should be inlined</div>;
}
 function App(props) {
  var a = props.x ? null : function() {};
  return a && <Child />;
}

In this case with, the React reconciler will see function () {} as one of the paths of the && condition and try and inline all paths and hit the fact you can't return function () {} and throw the ExpectedBailOut.

The realism is that the React reconciler in Prepack is being too smart here. It doesn't need to be concerned with the fact that the valid might not be valid. If the result is not valid, then the user will hit a runtime issue with React with their original input too. Ultimately, all we need to do is return the valid.

There are three tests attached to this PR. Only simple-24 is a direct test for the changes in this PR, simple-22 and simple-23 are regression tests for when #2494 is applied.

[gaearon]

Seems fine but I don't understand what the tests are checking

[gaearon]

Nit: just "in a condition" is sufficient. Also double space

Unless this is really specific to && and doesn't happen with if?

[trueadm]

It's specific to && which only occurs with Herman's PR applied. Where we have the the left-side of a && condition to evaluate to something thruthy, that is not a valid React return value. As it evaluates to true, the right-side will always be used.

[gaearon]

Nit: maybe drop #?

[gaearon]

I don't understand what this is supposed to test. What does <Child2 /> && <Child /> mean?

Judging by the issue description I would assume the test would be something like

function App(props) {
  if (props.neverHappens) {
    return <Bad />
  }
  return null;
}

function Bad() {
  return {}; // Invalid
}

and then the test should check we still render successfully.

[trueadm]

Will add this another test. The original tests from taken from my issue, test simple-23 fail with Herman's PR applied and simple-22 doesn't fully inline without Herman's PR.

[facebook-github-bot]

trueadm is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.