Components get hoisted outside of valid scope

[raulpineda]

Bug Report

Current Behavior
Using plugin-transform-react-constant-elements, when trying to render a component inside a map, the ref gets hoisted outside of the valid scope if said component doesn't include the current identity or tries to access a property of an object directly

Input Code

// @flow
import _ from "lodash";
import * as React from "react";
import AnotherComponent from "./anotherComponent";

const SPECIAL_KEYS = ["aKey", "anotherKey"];

class MyComponent extends React.PureComponent {
  static defaultProps = { 
    isLoaded: false,
  	aCondition: true,
  };
  
  constructor(props) {
    super(props);
  }

  render() {
    const { someProp, anotherProp } = this.props;

    let content;

    if (isLoaded) {
      const { anArray, problematicProp } = someProp;

      content = (
        <div>
          {CostCalculationAssumptions && (
            <ul>
              {_.map(anArray, key => (
                <li key={key}>
                  {assumptionLabel}
                  {_.includes(SPECIAL_KEYS, key) && aCondition ? (
                    //This one gets hoisted outside of if(isLoaded)
                    <AnotherComponent
                      value={problematicProp}
                      anotherValue={someProp}
                    />
                  ) : (
                    // This one doesn't get hoisted
                    <AnotherComponent
                      value={problematicProp}
                      anotherValue={someProp.foo}
                    />
                    // This wouldn't be hoisted either
                    // <AnotherComponent
                    //   value={problematicProp}
                    //   anotherValue={someProp}
                    //   key={key}
                    // />
                  )}
                </li>
              ))}
            </ul>
          )}
        </div>
      );
    }

    return <div>{content}</div>;
  }
}

export default MyComponent;

gets transformed into

"use strict";

Object.defineProperty(exports, "__esModule", {
  value: true
});

var _createClass = function () { function defineProperties(target, props) { for (var i = 0; i < props.length; i++) { var descriptor = props[i]; descriptor.enumerable = descriptor.enumerable || false; descriptor.configurable = true; if ("value" in descriptor) descriptor.writable = true; Object.defineProperty(target, descriptor.key, descriptor); } } return function (Constructor, protoProps, staticProps) { if (protoProps) defineProperties(Constructor.prototype, protoProps); if (staticProps) defineProperties(Constructor, staticProps); return Constructor; }; }();

var _lodash = require("lodash");

var _lodash2 = _interopRequireDefault(_lodash);

var _react = require("react");

var React = _interopRequireWildcard(_react);

var _anotherComponent = require("./anotherComponent");

var _anotherComponent2 = _interopRequireDefault(_anotherComponent);

function _interopRequireWildcard(obj) { if (obj && obj.__esModule) { return obj; } else { var newObj = {}; if (obj != null) { for (var key in obj) { if (Object.prototype.hasOwnProperty.call(obj, key)) newObj[key] = obj[key]; } } newObj.default = obj; return newObj; } }

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

function _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError("Cannot call a class as a function"); } }

function _possibleConstructorReturn(self, call) { if (!self) { throw new ReferenceError("this hasn't been initialised - super() hasn't been called"); } return call && (typeof call === "object" || typeof call === "function") ? call : self; }

function _inherits(subClass, superClass) { if (typeof superClass !== "function" && superClass !== null) { throw new TypeError("Super expression must either be null or a function, not " + typeof superClass); } subClass.prototype = Object.create(superClass && superClass.prototype, { constructor: { value: subClass, enumerable: false, writable: true, configurable: true } }); if (superClass) Object.setPrototypeOf ? Object.setPrototypeOf(subClass, superClass) : subClass.__proto__ = superClass; }

var SPECIAL_KEYS = ["aKey", "anotherKey"];

var MyComponent = function (_React$PureComponent) {
  _inherits(MyComponent, _React$PureComponent);

  function MyComponent(props) {
    _classCallCheck(this, MyComponent);

    return _possibleConstructorReturn(this, (MyComponent.__proto__ || Object.getPrototypeOf(MyComponent)).call(this, props));
  }

  _createClass(MyComponent, [{
    key: "render",
    value: function render() {
      var _props = this.props,
          someProp = _props.someProp,
          anotherProp = _props.anotherProp;


      var content = void 0;

      var
      //This one gets hoisted outside of if(isLoaded)
      _ref = React.createElement(_anotherComponent2.default, {
        value: problematicProp,
        anotherValue: someProp
      });

      if (isLoaded) {
        var anArray = someProp.anArray,
            problematicProp = someProp.problematicProp;


        content = React.createElement(
          "div",
          null,
          CostCalculationAssumptions && React.createElement(
            "ul",
            null,
            _lodash2.default.map(anArray, function (key) {
              return React.createElement(
                "li",
                { key: key },
                assumptionLabel,
                _lodash2.default.includes(SPECIAL_KEYS, key) && aCondition ? _ref :
                // This one doesn't get hoisted
                React.createElement(_anotherComponent2.default, {
                  value: problematicProp,
                  anotherValue: someProp.foo
                })
                // This wouldn't be hoisted either
                // <AnotherComponent
                //   value={problematicProp}
                //   anotherValue={someProp}
                //   key={key}
                // />

              );
            })
          )
        );
      }

      return React.createElement(
        "div",
        null,
        content
      );
    }
  }]);

  return MyComponent;
}(React.PureComponent);

MyComponent.defaultProps = {
  isLoaded: false,
  aCondition: true
};
exports.default = MyComponent;

Expected behavior/code
I would expect both cases to create the reference inside the if(isLoaded) {} block, since problematicProp is only declared there

Babel Configuration (.babelrc, package.json, cli command)

{
    "presets": [
        ["env", { "loose": true, "modules": false }]
    ],
    "plugins": [
        "transform-react-constant-elements",
    ]
}

Environment

• Babel version(s): 6.26.0
• Node/npm version: Node 8.8.1, npm 6.1.0
• OS: OSX 10.12.6
• Monorepo yes
• How you are using Babel: loader

[babel-bot]

Hey @raulpineda! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

[lukeapage]

I wonder if this comment is wrong:

https://github.com/babel/babel/blob/6.x/packages/babel-plugin-transform-react-constant-elements/src/index.js#L41

the babel hoister makes a mistake when hoisting it.

If the used prop is already var instead of let the symbol would be present but not assigned. I wonder if the hoister checks for last assignment (or maybe it checks last assignment and doesn't check destructuring)

[ArgonAlex]

I can confirm this still happens on Babel 7 too. Here's a minimal reproducible example using just @babel/preset-react and @babel/plugin-transform-react-constant-elements:
Input:

let element;
let content;
(() => {
  content = 1;
  element = <div>{content}</div>;
})()
console.log(element)

Output:

let element;
let content;

var _ref =
/*#__PURE__*/
React.createElement("div", null, content);

(() => {
  content = 1;
  element = _ref;
})();

console.log(element);

For me, it's happening whenever there is a closure, and the element only references variables declared outside of the function.

[ArgonAlex]

It looks like this might be fixed by #10529?

[nicolo-ribaudo]

Yup, thanks!