transform-react-constant-elements => Element type is invalid

[slorber]

After enabling this Babel react optimization I am getting the following error:

Error: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in.

If I disable just this plugin it works fine. My current test setup is:

Input Code

import React from "react";

const HOC = component => component;

const Parent = ({}) => (
  <div className="parent">
    <Child/>
  </div>
);

export default Parent;

let Child = () => (
  <div className="child">
    ChildTextContent
  </div>
);
Child = HOC(Child);

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

{
  "presets": [
    "react-app",
    "stage-0"
  ],
  "env": {
    "development": {
      "plugins": [
        "transform-react-constant-elements",
        "transform-react-inline-elements",
        "transform-react-remove-prop-types",
        "transform-react-pure-class-to-function"
      ]
    }
  },
  "sourceMaps": false
}

{
    "babel-core": "6.21.0",
    "babel-loader": "6.2.10",
    "babel-plugin-transform-react-constant-elements": "^6.9.1",
    "babel-plugin-transform-react-inline-elements": "^6.8.0",
    "babel-plugin-transform-react-pure-class-to-function": "^1.0.1",
    "babel-plugin-transform-react-remove-prop-types": "^0.2.11",
    "babel-preset-react-app": "2.0.1",
    "babel-preset-stage-0": "6.16.0"
}

Expected Behavior

It should probably work out of the box, eventually not optimizing things if they are too complicated to optimize, but should not require the user to rewrite his own code (IMO)

Current Behavior

The compiled output looks like that:

	var HOC = function HOC(component) {
	  return component;
	};

	var _ref2 = _jsx("div", {
	  className: "parent"
	}, void 0, _jsx(Child, {}));

	var Parent = function Parent(_ref) {
	  _objectDestructuringEmpty(_ref);

	  return _ref2;
	};

	exports.default = Parent;

	var _ref3 = _jsx("div", {
	  className: "child"
	}, void 0, "ChildTextContent");

	var Child = function Child() {
	  return _ref3;
	};
	Child = HOC(Child);

You can see that _ref2 is trying to use the Child component, which is declared after the _ref2 creation.

This is due to the HOC usage on the Child component, because comenting the HOC usage produces the following code that works fine:

	var HOC = function HOC(component) {
	  return component;
	};

	var Parent = function Parent(_ref) {
	  _objectDestructuringEmpty(_ref);

	  return _jsx("div", {
	    className: "parent"
	  }, void 0, _jsx(Child, {}));
	};

	exports.default = Parent;

	var _ref2 = _jsx("div", {
	  className: "child"
	}, void 0, "ChildTextContent");

	var Child = function Child() {
	  return _ref2;
	};
	//Child = HOC(Child);

Unfortunatly I'd like to use an HOC on the child. I think the plugin should support this usecase correctly to avoid users to have to rewrite any code. On large codebase this rewrite would be painful for users, and the React error is not helpful as-is: you have to introspect the stacktrace to find the source of the problem.

Possible Solution

The plugin could bypass the optimization in this usecase, exactly like it's done for when the HOC is used.

I think it makes sense to not optimize in this usecase because the plugin already doesn't optimize when using Child component without the HOC (probably because it's not safe), so I don't see why it would be safer once used through a HOC.

In case this would be safe to optimize, another possible solution is that instead of using static references, the plugin could output memoized functions that return the constant element, so that the constant element initialisation is deferred after app construction.

The output code could look like

       var _ref2 = memoize(function() {
         return _jsx("div", {
	   className: "parent"
	 }, void 0, _jsx(Child, {}));
       }

	var Parent = function Parent(_ref) {
	  _objectDestructuringEmpty(_ref);

	  return _ref2();
	};

As a side note, I would say that generalizing this memorisation technique in the plugin may lead to an app that could be faster on initial load, because only the required constant elements would be constructed on startup. But there would be a performance penalty after initial load due to using a memoized function call. This may be better or not according to the usecase (probably better in large apps that are not using code-splitting) and could be made a plugin option.

[babel-bot]

Hey @slorber! 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.

[slorber]

Mentionning some people that might be able to contribute: @thejameskyle @hzoo @kittens @STRML @appden @spicyj @DrewML @Timer @koba04

[danez]

Please don't ping random people, that won't make the resolution of the problem faster.

[slorber]

@danez these are not random people but people that previously contributed to this plugin or had discussed useful informations about it. About issue resolution I'm pinging people to know the right thing to do, and will likely make a PR if I am able to, based on feedback of pinged people.

[STRML]

Thanks @slorber, will take a look at this; seems like another issue with Hoister bindings.

[STRML]

Can confirm the following erroneous output:

import React from "react";

const HOC = component => component;

var _ref = <div className="parent">
    <Child />
  </div>;

const Parent = ({}) => _ref;

export default Parent;

var _ref2 = <div className="child">
    ChildTextContent
  </div>;

let Child = () => _ref2;
Child = HOC(Child);

Note Child's use in _ref before declaration.

[STRML]

This seems related to #3596, which prevented from hoisting to scopes where a binding hasn't been evaluated yet, but it did not seem to be able to handle situations where Program scope is all we have and we just need to reorder.

[slorber]

thanks @STRML that was fast!

Is reordering always possible? I mean there might be cases where people are using cyclic dependencies between the components? I've already done some things like that in my app.

This is a dumb question but what's the easiest way for me to test your PR against my app? It seems npm install does not support installing through git subdirectories and babel is a monorepo

[STRML]

You'd have to check out and build the whole project then copy the package out. If you have any patterns you think would break this, could you please post them so I can get them in the test suite?

…

On Jan 19, 2017 3:54 AM, "Sébastien Lorber" ***@***.***> wrote: thanks @STRML <https://github.com/STRML> that was fast! Is reordering always possible? I mean there might be cases where people are using cyclic dependencies between the components? I've already done some things like that in my app. This is a dumb question but what's the easiest way for me to test your PR against my app? It seems npm install does not support installing through git subdirectories and babel is a monorepo — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5149 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABJFPyUKllRHnRetSCg2dQLlSvSuQSlCks5rTzLfgaJpZM4LmANX> .

[slorber]

Hi,

@STRML it's probably better than before but it still does not work in my app. However it's probably very close to working because simply commenting a single HOC in my app makes it start successfully (didn't try to use it further but the basics are working)

I can try to make a simpler repro case, in the meantime here's my raw problematic source file: https://gist.github.com/slorber/c5093d2759815d6235aeb2a4e2fc0e2e
There's probably a lot of noise but I've left it totally unchanged in case it may be useful

Commenting these lines makes my app work:

WifiProblem = connect(state => ({
  connectionDown: AppCommonStore.connectionDownSelector(state),
}))(WifiProblem);

Tell me if you are able to do something or if you want me to look further

Note that for legacy reasons we are using a legacy class constructor: AtomReact.createPureClass(): don't know if this can be related

[STRML]

Did you try the code from the PR? I have compiled your repro and it seems fine, unless I'm missing something in the noise.

[slorber]

@STRML yes that's what I explain in my previous comment: I have tested your PR and I am still unable to start my app, due to a single file. I've posted a gist to this file unmodified so that you might be able to see where the problem come from. If you want I can try to get a smaller repro case

[slorber]

Maybe I've not tested correctly, I'll try again because your output seems fine. I'll see if I get the same output as you do.

[slorber]

@STRML I've just pasted your babel output to my app and it seems to work, so it's probably my test that was not good.

[STRML]

Okay; then this is okay to mark as fixed by #5153 and let's get it merged.

[STRML]

If you would, please keep this open until #5153 is merged (as it is not yet fixed in master).

[slorber]

so I'm not sure what do you want me to do :) I'll let this open until the PR is merged