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

mangle-props and Object.defineProperty #869

Closed
philipbulley opened this issue Nov 20, 2015 · 3 comments · Fixed by #3059
Closed

mangle-props and Object.defineProperty #869

philipbulley opened this issue Nov 20, 2015 · 3 comments · Fixed by #3059

Comments

@philipbulley
Copy link

The following is pre-uglified:

var d = Date.prototype;
Object.defineProperty(d, "year", {
  get: function () {
    return this.getFullYear()
  },
  set: function (y) {
    this.setFullYear(y)
  }
});

function demo() {
  var now = new Date;
  console.log(now.year); // 2000
  now.year = 2001; // 987617605170
  console.log(now);  // Wed Apr 18 11:13:25 GMT-0700 (Pacific Daylight Time) 2001
}

demo();

Which is uglified using:

uglifyjs --compress --beautify --mangle --mangle-props test.js

And produces:

function demo() {
    var e = new Date();
    console.log(e.c), e.c = 2001, console.log(e);
}

var d = Date.prototype;

Object.defineProperty(d, "year", {
    a: function() {
        return this.getFullYear();
    },
    b: function(e) {
        this.setFullYear(e);
    }
}), demo();

The now.year getter/setter has become e.c, but the string "year" in Object.defineProperty() remains. Surely "year" should become "c"?

@fabiosantoscode
Copy link
Contributor

This is going to be a tough one to fix. defineProperty creates a property and takes a string. That string needs to be changed, but uglifyjs doesn't always know how to mangle the string.

Another point is that the get and set names shouldn't be mangled either.

What I think of this second point is that this really shows why you really need to use --mangle-props together with a regexp. There are lots of APIs which won't take these new custom objects. Still needs a fix though.

@rvanvelzen
Copy link
Collaborator

This is why closure compiler never changes x["foo"] to x.foo - it allows the author to do this kind of stuff.

Generally, when using defineProperty, you should either not mangle props or specify a regex.

We could try to do sneaky stuff like automatically adding the second argument of a defineProperty call to the excluded props, but that seems way too magic-y.

@philipbulley
Copy link
Author

That's pretty much what I've ended up doing. In case it helps anyone in future, I first whitelist a bunch of properties as an array. Then blacklist/remove from that array any properties that have been implemented using defineProperty:

js = fs.readFileSync('main.js', 'utf8');
regex = /Object.defineProperty\(\s?.+?,\s?"(.+?)"/g
while(match = regex.exec(js)){
    // Ensure match[1] won't appear in my mangle regex
}

Note: That particular regex works well with the JS output from the Typescript compiler.

alexlamsl added a commit to alexlamsl/UglifyJS that referenced this issue Apr 6, 2018
alexlamsl added a commit that referenced this issue Apr 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants