babel-polyfill fails with TypeError: "define is not a function"

[izaera]

The module babel-polyfill@6.26.0/lib/index.js has the following code:

  function define(O, key, value) {
    O[key] || Object[DEFINE_PROPERTY](O, key, {
      writable: true,
      configurable: true,
      value: value
    });
  }

  define(String.prototype, "padLeft", "".padStart);
  define(String.prototype, "padRight", "".padEnd);

but because, when wrapping the modules for AMD loader, we add this on top of the module:

  var define = undefined;

the JS engine cannot find define.

Note that this is a very exotic case because var define = undefined; covers all cases except for this one.

In effect, if define is already present at the global scope, var define = undefined; masks it.

And in the case of modules that define a define variable, everything is OK because if you have this code:

  var define = undefined;
  console.log(define);
  var define = 'perico';
  console.log(define);

you get:

undefined
perico

as expected, and the JS engine doesn't complain about duplicate definitions.

However, it seems that in the case of variable vs function, the variable wins and thus babel-polyfill fails.

[izaera]

It seems that it's not that in variable vs function variable wins, but in this case, because of hoisting one is overwriting the other.

See https://stackoverflow.com/questions/336859/var-functionname-function-vs-function-functionname#comment16104094_336859

[izaera]

Given that it is a hoisting problem, doing this (putting the module code in a block after the var define = undefined;) solves the issue:

  var define = undefined;

  {
    function define(O, key, value) {
      O[key] || Object[DEFINE_PROPERTY](O, key, {
        writable: true,
        configurable: true,
        value: value
      });
    }

    define(String.prototype, "padLeft", "".padStart);
    define(String.prototype, "padRight", "".padEnd);
  }

I can't think of any pitfall in this construct, but I say that same sentence every time I touch this... :shrug:

[wincent]

I can't think of any pitfall in this construct

That makes two of us. About the only thing I can imagine is some overzealous minifier deciding that you didn't need the block and stripping it out.

[izaera]

:thinking: That shouldn't happen during the build because the bundler is usually the last step to run. Though you could theoretically configure such a setup.

However, we have a minifier in the portal that could do it.

However, if it did we would have seen it already, as the previous code was subject to that error too.

So I conclude it has never happened, but it's good to be aware of it in case some day happens (and I'm sure that know that we know it, it will happen for every existing installation out there :facepalm: )

[jbalsas]

🤔 That shouldn't happen during the build because the bundler is usually the last step to run. Though you could theoretically configure such a setup.

Remember about MinifierUtil in DXP...

[izaera]

@jbalsas That is the second part:

However, we have a minifier in the portal that could do it. However, if it did we would have seen it already, as the previous code was subject to that error too. So I conclude it has never happened, but it's good to be aware of it in case some day happens (and I'm sure that know that we know it, it will happen for every existing installation out there )

[wincent]

some overzealous minifier

Thankfully, Liferay wisely chose MinifierUtil over its more aggressive cousin, OverzealousMinifierUtil. OMU sounds better on paper ("10% more compression!"), but it always comes back to bite you in the end.