Node-serialport: Legacy arguments object pattern causing problem when code is compressed.

[rwaldron]

• __SerialPort Version__: 5.0.0-beta8
• __NodeJS Version__: 6.10.3
• __Operating System__ and __Hardware Platform__: Tessel 2

Summary of Problem

When t2-cli compresses JS code with uglify-es to bundle and deploy on Tessel 2, the code is compressed using the ecma: 6 compress option as well as the ecma: 6 output option. Thankfully, this results in very aggressive es6-centric compression, but unfortunately it also reveals "gotchas" of using es6 era features with legacy patterns. There is a problem pattern in lib/util.js, that when compressed, produces this:

(I've obviously run the code through jsbeautifier for readability)

"use strict";
module.exports = {
  promisify: o => {
    if ("function" != typeof o) throw Error('"func" must be a function');
    return () => new Promise((r, t) => {
      const e = Array.prototype.slice.apply(arguments);
      console.log(e[0]), e.push((o, e) => {
        if (o) return t(o);
        r(e)
      }), o.apply(null, e)
    })
  }
};

It may not be obvious at first, but the problem is that there is actually _no_ arguments object in arrow functions:

But uglify-es isn't going to change your original arrow function in the Promise instantiation:

'use strict';

function promisify(func) {
  if (typeof func !== 'function') {
    throw new Error('"func" must be a function');
  }
  return function() {
    return new Promise((resolve, reject) => {
      const args = Array.prototype.slice.apply(arguments);
      args.push((err, data) => {
        if (err) {
          return reject(err);
        }
        resolve(data);
      });
      func.apply(null, args);
    });
  };
}

module.exports = {
  promisify
};

Because it's _totally_ valid to access the arguments object of a function that an arrow function is nested inside of. The great news is that this requires only a minor change to fix! All that needs to be done is for the arguments object to move out of the nested arrow function and into the actual function from which it originated, then uglify-es will know to do the right thing and preserve that function's _FunctionExpression_ syntax.

Patch to follow!

[rwaldron]

Sorry, I forgot to show the source change and corresponding compressed code:

'use strict';

function promisify(func) {
  if (typeof func !== 'function') {
    throw new Error('"func" must be a function');
  }
  return function() {
    const args = Array.from(arguments);
    return new Promise((resolve, reject) => {
      args.push((err, data) => {
        if (err) {
          return reject(err);
        }
        resolve(data);
      });
      func.apply(null, args);
    });
  };
}

module.exports = {
  promisify
};

Results in:

"use strict";
module.exports = {
  promisify: r => {
    if ("function" != typeof r) throw Error('"func" must be a function');
    return function() {
      const n = Array.from(arguments);
      return new Promise((t, o) => {
        n.push((r, n) => {
          if (r) return o(r);
          t(n)
        }), r.apply(null, n)
      })
    }
  }
};

(Again, I ran the code through jsbeautifier for readability)

[rwaldron]

Also, that's the only place in the code that this occurs

[reconbot]

Who would have _thunk_? You make some good _arguments_. I _promise_ don't mind merging any patches that _slice_ through this ... aaannd I'm out of puns.

[rwaldron]

😆 🤣

[rwaldron]

Fixed!

[mehalshah]

Alright guys, time to _packet_ in

[getify]

Just curious (side note at best)... why does it produce this:

module.exports = {
  promisify: o => {
...

instead of:

module.exports = {
  promisify(o) {
...