butternut icon indicating copy to clipboard operation
butternut copied to clipboard

Unused identifiers in object/array patterns are not removed

Open Rich-Harris opened this issue 7 years ago • 8 comments

Edited: no longer a bug, but suboptimal output

// input
(function () {
  var {foo,bar} = obj;
  console.log(foo);
}())

// expected
!function(){var {foo:a}=obj;console.log(a)}()

// or maybe
!function(){console.log(obj.foo)}()

// actual
!function(){var {foo:a,bar:b}=obj;console.log(a)}()

Rich-Harris avatar May 10 '17 03:05 Rich-Harris

Wouldn't the better output be?

!function(){console.log(obj.foo)}()

That would be more along the lines of what Closure Compiler does but it would be awesome if butternut could too :)

trueadm avatar May 10 '17 15:05 trueadm

Yes, but one thing at a time 😀 That would be a separate optimisation that determined a) if a variable was only used once, and b) whether it would be safe to inline it. Not specifically related to destructuring.

Incidentally this is no longer a bug (can try it out on https://butternut.now.sh/) — it now gives this result...

!function(){var{foo:a,bar:b}=obj;console.log(a)}()

...which is correct, but suboptimal. Working on eliminating unused identifiers inside object/array patterns.

Rich-Harris avatar May 10 '17 15:05 Rich-Harris

(Incidentally, Closure generates code like you suggest; Uglify doesn't)

Rich-Harris avatar May 10 '17 15:05 Rich-Harris

@Rich-Harris could removing unused identifiers in objects cause performance deopts due to monomorphism?

trueadm avatar May 10 '17 15:05 trueadm

Not talking about removing identifiers in objects, just in object patterns. Do you know if that has the same performance implications?

Rich-Harris avatar May 10 '17 15:05 Rich-Harris

@Rich-Harris I thought you meant you'd remove bar from the destructured object, I guess you just meant the alias to b would be removed?

trueadm avatar May 10 '17 15:05 trueadm

Am saying that in this situation...

// current
!function(){var{foo:a,bar:b}=obj;console.log(a)}()

// better?
!function(){var{foo:a}=obj;console.log(a)}()

...we're not affecting the shape of obj in any way, we're just not creating a local identifier for obj.bar because it's unused. I'd assume that that doesn't have performance implications, but I'm not certain.

Rich-Harris avatar May 10 '17 15:05 Rich-Harris

@Rich-Harris That's what I originally meant, I'm not sure either – I thought it may have had an affect. I'll probably do some benchmarks on that actually sometime, I'm interested in knowing now. Either way, good work on getting this bug fixed :)

trueadm avatar May 10 '17 15:05 trueadm