Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: destructuring + calling function with same param name as destructured var #3122

Closed
xaviergonz opened this issue May 4, 2018 · 7 comments

Comments

@xaviergonz
Copy link

Bug report

Uglify version (uglifyjs -V) latest

JavaScript input

(function() {

const animReq = (data) => {
  const { newRotation, character } = data;
  console.log('')
  const rotationDuration = getRotationDuration(newRotation, character.rotation);
  console.log('')
  return rotationDuration;
}

function getRotationDuration(newRotation, oldRotation) {
  return getRotationDeltaDuration(newRotation - oldRotation);
}

function getRotationDeltaDuration(rotationDelta) {
  const durationMultiplier = Math.round(Math.abs(rotationDelta)) / 180;
  if (durationMultiplier === 0) {
    return 0;
  }
  return lerp(150, 450, durationMultiplier);
}

function lerp(v0,v1,t) {
  return v0 * (1 - t) + v1 * t;
}


animReq({
  newRotation: 10,
  character: {
    rotation: 100
  }
});

})()

The uglifyjs CLI command executed or minify() options used.

npx uglify-es --compress -- test.js

JavaScript output or error produced.

The output is the following:

!function(){var newRotation,oldRotation;(data=>{const{newRotation:newRotation,character:character}=data;console.log("");const rotationDuration=(newRotation=newRotation,oldRotation=character.rotation,function(rotationDelta){const durationMultiplier=Math.round(Math.abs(rotationDelta))/180;return 0===durationMultiplier?0:150*(1-(t=durationMultiplier))+450*t;var t}(newRotation-oldRotation));console.log("")})({newRotation:10,character:{rotation:100}})}();

The problem is with the sequences compress option, which generates this part:

const{newRotation:newRotation,character:character}=data;
...
const rotationDuration=(newRotation=newRotation,...

where newRotation=newRotation fails because newRotation is const and cannot be reassigned

@xaviergonz
Copy link
Author

PS: if the function argument is changed to something other than the destructured var name (e.g.

const { newRotation, character } = data;
...
function getRotationDuration(newRotation2, oldRotation) {

it works since the generated expression is then newRotation2=newRotation

@kzc
Copy link
Contributor

kzc commented May 4, 2018

@xaviergonz
Copy link
Author

There are two uglify-es packages? What's the difference?

@kzc
Copy link
Contributor

kzc commented May 4, 2018

@xaviergonz
Copy link
Author

xaviergonz commented May 4, 2018

Sorry if this sounds like a silly question but, why aren't both packages joined then with more contributors? The latest commit to this one (I presume the official one) is only 11h old.

I ask because I'm actually using using a bundler (fusebox) that depends on this uglify-es package internally and I think the only way I could get it to work is to make a yarn alias from one to the other, which feels weird.

@kzc
Copy link
Contributor

kzc commented May 4, 2018

One is actively maintained and the other is not. Use whatever package you prefer.

@alexlamsl
Copy link
Collaborator

Fixed in #2881

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants