Certain function inlining not maintaining a function reference

[molily]

Bug report or Feature request?

Bug

Version (complete output of terser -V or specific git commit)

4.3.4

Complete CLI command or minify() options used

--compress --mangle

terser input

See https://molily.de/assets/bundle-before-terser.js

terser output or error

See https://molily.de/assets/bundle-after-terser.js

This is the bundle of a project with Preact, compiled with Webpack. The production build is minified with Terser.

bundle-before-terser.js contains the pre-minified code of Preact in line 1042.

Excerpt (prettified):

function preact_module_C(n, l, u, t, i) {
  var r;
  for (r in u) r in l || N(n, r, null, u[r], t);
  for (r in l)
    (i && "function" != typeof l[r]) ||
      "value" === r ||
      "checked" === r ||
      u[r] === l[r] ||
      N(n, r, l[r], u[r], t);
}
// …
function N(n, l, u, t, i) {
  var r, o, f, e, c;
  if (
    "key" ===
      (l = i
        ? "className" === l
          ? "class"
          : l
        : "class" === l
        ? "className"
        : l) ||
    "children" === l
  );
  // …
}
// …
function $(l, u, t, i, r, o, f, e, c, a) {
  var h,
    // …
    N = u.type;
  // …
  try {
    n: if ("function" == typeof N) {
    	// …
    } else u.__e = z(t.__e, u, t, i, r, o, f, a);
    // …
  } catch (l) {
    // …
  }
  return u.__e;
}
function z(n, l, u, t, i, r, o, c) {
  // …
  return (
    null === l.type
      ? p !== d && (null != r && (r[r.indexOf(n)] = null), (n.data = d))
      :
        // …
        c ||
          ((v || h) &&
            ((v && h && v.__html == h.__html) ||
              (n.innerHTML = (v && v.__html) || ""))),
        preact_module_C(n, d, p, i, c),
        // …
    n
  );
}

TL;DR:

• There are four relevant functions: N, preact_module_C, z, and $.
• preact_module_C is called only once, that is in z. So Terser inlines preact_module_C into z.
• z is called only once, that is in $. So Terser inlines z into $.
• preact_module_C calls N. So Terser needs to maintain that reference.
• $ has a local variable named N which shadows the N function in the outer scope.

After the minification by Terser, the inlined preact_module_C code looks like this:

(function(e, t, n, r, o) {
  var i;
  for (i in n) i in t || C(e, i, null, n[i], r);
  for (i in t)
    (o && "function" != typeof t[i]) ||
      "value" === i ||
      "checked" === i ||
      n[i] === t[i] ||
      C(e, i, t[i], n[i], r);
})(e, v, h, o, a),

The call to N is now a call to C.

Meanwhile, N has been renamed to E:

function E(e, t, n, r, o) {
  var i, u, a, c, s;
  if (
    "key" ===
      (t = o
        ? "className" === t
          ? "class"
          : t
        : "class" === t
        ? "className"
        : t) ||
    "children" === t
  );
  // …
}

So the reference is broken.

It is worth noting that, in fact, there is a C in the scope. But it’s a variable in the function j, formerly known as $, the new parent after the inlining.

function j(e, t, n, r, o, u, a, c, s, l) {
  var d,
    // …
    C = t.type;
   // …
}

C was formerly known as N. Maybe that is what confuses Terser: “Oh, there is an N already in the new scope chain, so I’ll update the reference to its new name”.

C is a string here, not a function. So the code produces a TypeError during runtime.

I think this confusion is a result of several function inlining operations.

I really tried to boil this down to a minimal test case for you, but if I take anything away, Terser correctly maintains the reference to the function originally called N.

Also, if I make a minimal setup with four functions like above, everything works fine:

(function() {
  function a() { console.log('4'); }
  function b() { console.log('3'); a(); console.log('5'); }
  function c() { console.log('2'); b(); console.log('6'); }
  function d() {
    // Shadow a
    var a = Math.sqrt(2);
    // Make sure there is a reference to the local a
    console.log(a);
    console.log('1'); c(); console.log('7');
  }
  d();
})();
// Output: 1.4142135623730951 1 2 3 4 5 6 7

Output:

!function() {
    function o() {
        console.log("3"), console.log("4"), console.log("5");
    }
    !function() {
        var l = Math.sqrt(2);
        console.log(l), console.log("1"), console.log("2"), o(), console.log("6"), console.log("7");
    }();
}();
// Output: same as above

o could be inlined again (and it is with another Terser run), but the code it correct.

Thanks for bearing with me. Let me know if you have questions or tips how I can make this any more helpful for you.

[fabiosantoscode]

Thank you very much for this detailed report! Not everyone finds the time to do some research on the bug before posting an issue :)

[mileszim]

We are having the same issue, involving a function inside jQuery.

The code as terser generates (broken):

If you look at the highlighted line, it appears terser is swapping e for s and then using s from then on, without assigning s = e. It seems to do so due to the line above, in which e is used a second time as a minified function argument nested in the parent function.

The code as uglify-js generates (working):

The original source:

[fabiosantoscode]

@mileszim please provide the actual code when reporting bugs, it helps a lot! If you were reproducing an error, you wouldn't like to type every character in a screenshot :)

[fabiosantoscode]

@molily I can't reproduce this anymore, with the following code:

curl https://molily.de/assets/bundle-before-terser.js -Ssl > bug.js
terser bug.js  -bmc  > bug-out.js

The code that's generated is correct:

// preact_module_C (inlined into z)
           function(e, t, n, r, o) {                                                                                                                                           
              var i;                                                                                                                                                          
              for (i in n) i in t || S(e, i, null, n[i], r);                                                                                                                  
              for (i in t) o && "function" != typeof t[i] || "value" === i || "checked" === i || n[i] === t[i] || S(e, i, t[i], n[i], r);                                     
          }(e, v, h, o, a)

// function N (renamed to S, as called above)
      function S(e, t, n, r, o) {                                                                                                                                             
          var i, u, a, c, s;                                                                                                                                                  
          if ("key" === (t = o ? "className" === t ? "class" : t : "class" === t ? "className" : t) || "children" === t) ; else if ("style" === t) if (i = e.style,           
          "string" == typeof n) i.cssText = n; else {                                                                                                                         
              if ("string" == typeof r && (i.cssText = "", r = null), r) for (u in r) n && u in n || C(i, u, "");                                                             
              if (n) for (a in n) r && n[a] === r[a] || C(i, a, n[a]);                                                                                                        
          } else "o" === t[0] && "n" === t[1] ? (c = t !== (t = t.replace(/Capture$/, "")),                                                                                             s = t.toLowerCase(), t = (s in e ? s : t).slice(2), n ? (r || e.addEventListener(t, E, c),                                                                                    (e.t || (e.t = {}))[t] = n) : e.removeEventListener(t, E, c)) : "list" !== t && "tagName" !== t && "form" !== t && !o && t in e ? e[t] = null == n ? "" : n : "funct  ion" != typeof n && "dangerouslySetInnerHTML" !== t && (t !== (t = t.replace(/^xlink:?/, "")) ? null == n || !1 === n ? e.removeAttributeNS("http://www.w3.org/1999/xlink",   t.toLowerCase()) : e.setAttributeNS("http://www.w3.org/1999/xlink", t.toLowerCase(), n) : null == n || !1 === n ? e.removeAttribute(t) : e.setAttribute(t, n));             
      }                                                                                                                                                                       

If I run the code in the browser (injecting an initialState span with [] in it so it doesn't crash JSON.parse), it renders a div that reads "hi".

Can you try running with the latest Terser version? (4.3.9) I might've fixed this issue while fixing something else.

[molily]

Thanks a lot for your efforts! Minifying the example file that only renders 'hi' now works. However, minifying my full app build still produces the same error. Here are the full files:

https://molily.de/assets/full-bundle-before-terser.js
https://molily.de/assets/full-bundle-after-terser.js

The reference to C is not correctly updated, line 837:

                    for (i in n) i in t || C(e, i, null, n[i], r);
                    for (i in t) o && "function" != typeof t[i] || "value" === i || "checked" === i || n[i] === t[i] || C(e, i, t[i], n[i], r);

where the wrong C is defined here, see line 794:

function j(e, t, n, r, o, u, a, c, s, l) {
        var d, v, y, m, g, w, O, P, T, E, C = t.type;

The correct function that should be called is now called E, see line 781:

function E(e, t, n, r, o) {
        var i, u, a, c, s;

By the way, the source code is here if you like to test with it: https://github.com/molily/universal-progressive-todos/tree/preact-no-minification (branch preact-no-minification). I’ve disabled the terser-webpack-plugin so npm run build produces a plain build at dist/client/static/client.js.

Let me know if I can help!

[fabiosantoscode]

After a couple of hours deleting code, I managed to reduce your pre-terser file to this test case:

"use strict";                                                                          
function $() {                                                                         
    var same_name = undefined;                                                         
    console.log(same_name === undefined ? "PASS" : "FAIL")                             
    indirection_1();                                                                   
}                                                                                      
function indirection_1() {                                                             
    return indirection_2()                                                             
}                                                                                      
function indirection_2() {                                                             
    for (const x of [1]) { same_name(); return; }                                      
}                                                                                      
function same_name() {                                                                 
    console.log('PASS')                                                                
}                                                                                      
$();                                                                                   

If the function order is different, this does not happen, which I find interesting.

[fabiosantoscode]

v4.6.2 should come out tomorrow with this fix. Sorry it took so long but it was harder than it seems to fix :)

[molily]

Thank you very much, it works like a charm! :)

[fabiosantoscode]

Super cool!