babel / minify

:scissors: An ES6+ aware minifier based on the Babel toolchain (beta)
https://babeljs.io/repl
MIT License
4.39k stars 225 forks source link

[feature request] Convert functions to arrow functions #354

Open FezVrasta opened 7 years ago

FezVrasta commented 7 years ago

I think Babili could save a lot of bytes with a plugin that converts functions that do not make use of this to arrow functions:

Input (34):

function foo() {
  return 'bar';
}

Output(18):

let foo=()=>'bar';
// or 20 with `const`

Current result: https://babeljs.io/repl/#?babili=true&evaluate=true&lineWrap=false&presets=es2015%2Creact%2Cstage-2&code=function%20foo()%20%7B%0A%20%20return%20'bar'%3B%0A%7D

boopathi commented 7 years ago

Cannot determine if new is called. So we cannot safely transform this.

kangax commented 7 years ago

We talked about this in the past. We can add this transform but not only would it be unsafe (so disabled by default) but it would also have to be marked as "es2015". Other transforms of this kind could include destructuring, default args, etc.

Pyrolistical commented 7 years ago

If the target was es2015, why is this transform unsafe if this isn’t used? I don’t understand the “we don’t know if new was called”

topaxi commented 7 years ago

@Pyrolistical arrow functions don't have a prototype property and therefore cannot be called with new.

Example:

function foo() {}
new foo() // not a problem
typeof foo.prototype === 'object'
const foo = () => {}
new foo() // TypeError: foo is not a constructor
typeof foo.prototype === 'undefined'
Andarist commented 7 years ago

Actually I think it could be determined (to some extent) if new is gonna be used. If resulting foo is local, not exported, not assigned to any object or reassigned it would be safe to apply such transformation. Wouldnt it?

Pyrolistical commented 7 years ago

Hmm, but even the most common case would not be transformed. For example

function FakePromise() {
  return {
    then(constructor) {
      return new constructor()
    }
  }
}

FakePromise()
  .then(function() {})

Promise.resolve()
  .then(function() {})

You wouldn't be able to tell the difference, so neither would be transformed

Andarist commented 7 years ago

Could be then hidden behind "unsafe" flag - just like uglify does it. Passing constructors around is not that common pattern in my practice.

Pyrolistical commented 7 years ago

yeah, but unsafe flags means most people would never use this feature and its not worth developing.

i would rather program a better static analyzer where if you can prove the closure is never newed then do the transform. it would be pretty tricky and you would need to add ability to recognize and follow promises.