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

Include an option to mangle private/all method names #103

Closed
ghost opened this issue Jan 21, 2013 · 44 comments
Closed

Include an option to mangle private/all method names #103

ghost opened this issue Jan 21, 2013 · 44 comments

Comments

@ghost
Copy link

ghost commented Jan 21, 2013

I think there should be an option (off by default) to mangle some/all method names:

In cases where the entire codebase is minified into one file, you should be able to mangle this.<<anything>>.

In cases where some methods are marked as "private" using the underscore prefix convention, you should be able to mangle this._<<anything>>. This will potentially break something, but since everything is "private" anyway, I find it to be an acceptable risk and so will many other people. Possibly have a regex here to determine which methods to mangle to allow for different coding conventions.

@MikePeterson
Copy link

+1 for renaming private properties with the '_' prefix. Currently I have to use another app to perform this action before running UglifyJS.

@jtangelder
Copy link

A better way is with JSDoc I suppose. I believe Google Closure is using this also.

/**
 * this method does nothing
 * @private
 */
function whatever() { 
    // really!
}

@ghost
Copy link
Author

ghost commented Jan 30, 2013

Would it be possible to also do this for entire objects?

/** @private **/
var obj = { someKey : 'foo', someOtherKey : 'bar' };

which would yield:

var a={b:'foo',c:'bar'};

...

fn(a.b);

Though I object to tacking onto something in JSDoc because although I'm perfectly okay obfuscating out a private method / object in my own code, it's very possible that doing this in general would break things because people do hook into and use things that would be "private" code in general libraries. I suppose as long as it's an opt-in flag in uglifyjs it would be ok?

Though I realize this entire concept is destructive, which uglifyjs is somewhat opposed to, so I'm not sure exactly how to handle it? It's just ugly seeing my "uglified" code look like:

this.__someMethod(this.__someProperty);

when it could be:

this.a(this.b);

@rtm
Copy link

rtm commented Jul 25, 2013

How could that possibly work, when the program could access "a[foo]" where the value of foo is computed?

@maherbeg
Copy link

@rtm If you want to access a property via a[foo] you should access it via a['foo'] with a string literal and assign the property to a string literal.

+1 for this. We can't switch from Closure to Uglify until this is an option. There is a 60KB difference in file size between the two right now.

@heff
Copy link

heff commented Dec 3, 2013

+1

@axkibe
Copy link

axkibe commented Jan 17, 2014

I'm also looking for this option. Yes its unsafe since
a.foo = 'bar';
console.log( a['fo' + 'o'] );

would break. or if an object would be JSONfied

But honestly, I'd be happy if I'd give a list of names I'd like to be mangled, since I know I'm not doing stuff like that with.

@axkibe
Copy link

axkibe commented Jan 18, 2014

I came up with a solution for myself:

http://jsfiddle.net/u9zYV/

It walks the tree after being parsed and replaces all tokens marked as OK to be mangled with a respective shortcut.

I got little idea how uglifyjs2 works internally so it may be a bit of naive approach.

It requires a manually maintained config file which marks all properties that are ok to be mangled and those that are not to be touched. Respective string literals will also be mangled! It assumes that no property is named anything starting with "$$" so far.

Maintaining that list is a bit tricky, since anything that interacts with the browser, becomes a JSON object to be interacted with the server or which is a string literal that can be seen by the user must not be mangled. To make this somewhat maintainable the list of properties not to be mangled is also managed and the tool warns about any properties that is in neither.

I'm wondering if distinguishing single and double quote string literals would be a nice idea for a given project. Like single quotes "this literal is okay to be mangled" and double quotes "do not touch this". However, as far I can see, the current parser drops this detail.

@mdsb100
Copy link

mdsb100 commented Jun 23, 2014

+1

@LekOkay
Copy link

LekOkay commented Dec 3, 2014

Nice 👍 Im looking for this options also.. Thank you :)

@flesler
Copy link

flesler commented Feb 6, 2015

+1 has been done for ages using Dean Edwards Packer

@kuus
Copy link

kuus commented Feb 6, 2015

+1
@MikePeterson may I ask you which app are you using?

I like both ideas: mangle prototype methods starting with underscore or marked as @Private with JSDocs syntax (like @jtangelder said). Would it be possible to mangle constructor properties as well with this right?
What I would really like to be possible is to mark a constructor as entirely private and obtain the following:

/**
 * @constructor
 * @private
 */
function Mangle(what, when) {
 this.whatToMangle = what;
 this.whenToMangle = when;
}
Mangle.prototype.doit = function () {
  return 'mangle ' + this.whatToMangle + ' on ' + this.whenToMangle;
}

// uglified to:

function M(w,h) { this.a = w; this.h = h; }
M.prototype.d = function () { return 'mangle ' + this.a + ' on ' + this.h; }

Is that doable?

@dwoldrich
Copy link

I'd like to request this feature as well, with a config parameter to allow for @Protected symbols to be mangled as well.

@jrhite
Copy link

jrhite commented Mar 29, 2015

The latest release of uglify (today) has object property mangling, see v2.4.18. It also supports reserved files for excluding both object properties and variables that you don't want mangled. Check it out.

@danvk
Copy link

danvk commented Mar 29, 2015

Is it possible to use this feature via the uglify-js library (not the command line tool)? I'm not seeing any version of it in node.js.

@jrhite
Copy link

jrhite commented Mar 30, 2015

Yes, just call it like this:

UglifyJS.mangle_properties(topLevel, { reserved:
arrayOfYourReservedPropertyNames, cache: arrayOfCachedProperties })

You only need to pass in cache if you want or are using the new
--name-cache feature. Same for reserved, it's optional but highly likely
you'll need it. (eg: at the very least, browser built-in DOM properties).
If you want to get a a pre-built list of properties for many browsers that
comes built-in with the tool by default call
Uglify.readDefaultReservedFile() to get reserved 'vars' and 'props'. It
will return an object like:

{

'vars': [...array of reserved vars...],

'props': [...array of reserved properties...]

}

Then just pass in the 'props' array to the call to
UglifyJS.mangle_properties()

On Mon, Mar 30, 2015 at 4:25 AM, Dan Vanderkam notifications@github.com
wrote:

Is it possible to use this feature via the uglify-js library (not the
command line tool)? I'm not seeing any version of it in node.js
https://github.com/mishoo/UglifyJS2/blob/3ec11c781b61efd53c001c05e8e52fbd1e6e9034/tools/node.js#L55
.


Reply to this email directly or view it on GitHub
#103 (comment).

@danvk
Copy link

danvk commented Mar 30, 2015

Thanks. Is there a reason this feature couldn't be done via the usual opts parameter to UglifyJS.minify? I'm using gulp-uglify, which just exposes the opts param. Sounds like I may need to file an issue against gulp-uglify to expose this?

@jrhite
Copy link

jrhite commented Mar 30, 2015

I'm not exactly sure, you'll have to ask the original author of uglify. I
didn't actually make the uglify changes. I think he wanted to keep property
mangling separate as it happens in a different stage than minification and
variable mangling.

Indeed, I worked on a PR
gruntjs/grunt-contrib-uglify#312 for
grunt-contrib-uglify to incorporate the new changes. The PR is still
pending, but you can take a look. It may help with gulp. You'll either need
to file an issue for gulp or prep a PR yourself.

On Mon, Mar 30, 2015 at 9:07 AM, Dan Vanderkam notifications@github.com
wrote:

Thanks. Is there a reason this feature couldn't be done via the usual opts
parameter to UglifyJS.minify? I'm using gulp-uglify
https://www.npmjs.com/package/gulp-uglify, which just exposes the opts
param. Sounds like I may need to file an issue against gulp-uglify to
expose this?


Reply to this email directly or view it on GitHub
#103 (comment).

@rcdilorenzo
Copy link

I agree with @danvk. There are other projects that expose the opts params and I would like to use this feature through the opts object as well.

@niemyjski
Copy link

+1

2 similar comments
@pieroxy
Copy link

pieroxy commented May 25, 2015

+1

@zengfenfei
Copy link

+1

@chemoish
Copy link

+1 train

@ffabreti
Copy link

+1 for mangle_properties on .minify()

@ffabreti
Copy link

@jrhite, could you give a full example of using mangle_properties programmatically? I tryied to use your sugestion (mar 29) but it didn't work and I can't figure it out from reading the source code.
Also, can I use <mangle-regex> option too?

@ulrikmoe
Copy link

+1

@jrhite
Copy link

jrhite commented Aug 29, 2015

The best working example (most-used?) is probably just to point directly to
the grunt plugin for uglify. Here are the specific lines of code you're
interested in:

https://github.com/jrhite/grunt-contrib-uglify/blob/master/tasks/lib/uglify.js#L120-L137

Cheers!

On Fri, Aug 28, 2015 at 10:59 PM, Ulrik Moe notifications@github.com
wrote:

+1


Reply to this email directly or view it on GitHub
#103 (comment).

@jrhite
Copy link

jrhite commented Aug 29, 2015

Actually, oops...just pointed you to my local copy of the code which might
be a little stale. Better to point you to the official code:

https://github.com/gruntjs/grunt-contrib-uglify/blob/master/tasks/lib/uglify.js#L120-L134

On Sat, Aug 29, 2015 at 1:44 PM, Jacob Hite jrhite@gmail.com wrote:

The best working example (most-used?) is probably just to point directly
to the grunt plugin for uglify. Here are the specific lines of code you're
interested in:

https://github.com/jrhite/grunt-contrib-uglify/blob/master/tasks/lib/uglify.js#L120-L137

Cheers!

On Fri, Aug 28, 2015 at 10:59 PM, Ulrik Moe notifications@github.com
wrote:

+1


Reply to this email directly or view it on GitHub
#103 (comment).

@IngwiePhoenix
Copy link

+1 for this. It seems that stuff like WebPack and some other minifiers relying on Uglfy do not support this option, leaving me with no way to properly mangle things...

@mbrevda
Copy link

mbrevda commented Sep 3, 2015

+1

@myadzel
Copy link

myadzel commented Dec 25, 2015

--mangle-props --mangle-regex='/^_/'

@zengfenfei
Copy link

+1

@fabiosantoscode
Copy link
Contributor

Since --mangle-props --mangle-regex was shipped, should this issue be closed? Or do we want to implement /** @private **/ as well?

@kuus
Copy link

kuus commented Mar 14, 2016

Since --mangle-props --mangle-regex was shipped, should this issue be closed? Or do we want to implement /** @Private **/ as well?

for what it worth, personally I would love to see that implemented as well!

@fabiosantoscode
Copy link
Contributor

I just realised that we can't implement /** @private **/. We'd need to perform flow analysis. Take this example:

var x = {
   /** @private **/
   longPrivateName: 6
};
console.log(x.longPrivateName)

When compressing the argument to console.log, we have no idea of what type x is, or where the property longPrivateName was defined, so no way to find out whether it was defined with @private.

We may be able to reach it by looking at the symbol's definition, but once you go cross-function, cross-module, or global, things fall apart.

@objectkit
Copy link

I am just throwing this out there, warts and all. I too want to be able to comment members of objects as private, as too with variables in general, but I would be content with private scope limited to declaring objects.

As I see the example above, correct, longPrivateName would be private to the declaring object, x, so any attempt to access it outside of the object context of x would of course fail. It would fail for logical reasons as the member would not be accessible has it would have a different name at runtime. If the scope of private was, say, to all referents in a declaring file for example, then I would argue that would not be private in the conventional sense. However, if private were limited to scope of the declaring object, and say 'x' also had a method called 'getPrivateName", then

var x = {
    /** @private */
    longPrivateName: 6
,   getLongPrivateName: function () { return this.longPrivateName }
}

console.log(x.getLongPrivateName())

would work, even it had been compressed to something like

var x = {q:6,w: function () { return this.q}};
console.log(x.w())

would work.

I should really think about this more before posting, so forgive if me if I missed the overall context of what was said there. I think if private was absolutely strict about access to object members then private declarations would work without flow analysis... without flow analysis, it would break, as would be intended, no?

@fabiosantoscode
Copy link
Contributor

In your above example, this line:

x.w()

To do this you need flow analysis. At the point that this call is being mangled, uglifyjs doesn't know what object x is. So you may have:

var x = {
  /** @private **/
  height: 100
};
var y = {
  height: 100
};
return y.height;

(notice that the second is not private)

Which is a very basic set-up, but since uglify doesn't know where this .height is coming from (it may be coming from the DOM API even), it can't know whether or not to mangle.

The current solution of using --mangle-regex pretty well if you enforce a private naming convention. If I say "everything that starts with a _ is private" we don't need flow analysis because all the information uglifyjs needs is written in the name.

@alekseychikin
Copy link

If i use --mangle constructions like that:

    1: [ function(require, module, exports) {
        module.exports = function() {

gonna translate to something like that:

    1: [ function(a, b, c) {
        b.exports = function() {

(this is compiled code of browserify)

If i use --mangle-props instead this trick won't work. But if i use --mangle-regex than an option --mangle just ignored. But it works with --mangle-props.

So! If there is the way to mix --mangle and --mangle-regex for private methods i would be sooo happy.

@hakatashi
Copy link

hakatashi commented May 12, 2016

Just an idea, but I think this feature can be implemented by 100%-compatibility with the original code. That is, replace any call for properties with string literals and mangle long string literals to variables.

An example follows.

Original code:

var foo = 'length';
var bar = {
    baz: 'boo',
    length: 10,
};

if ('string'.length === foo.length) {
    bar['length'] = bar.baz.length;
}

Replace all calls for property names with string literal:

var foo = 'length';
var bar = {
    baz: 'boo',
    length: 10,
};

if ('string'['length'] === foo['length']) {
    bar['length'] = bar['baz']['length'];
}

Then search for all occurrence of string literals in code, and mangle them with variables.

var a = 'length', b = 'boo', c = 'string', d = 'baz';

var foo = a;
var bar = {
    baz: b,
    length: 10,
};

if (c[a] === foo[a]) {
    bar[a] = bar[d][a];
}

.length will become [a]... it's fairly shortened from 7-chars to 3-chars.

@shaharmor
Copy link

Any update on this?

@rvanvelzen
Copy link
Collaborator

@skaharmor Your question is a bit too broad. You can already let UglifyJS mangle properties based on a regex - https://github.com/mishoo/UglifyJS2#mangleproperties-options

@fabiosantoscode
Copy link
Contributor

I think this issue should be closed, advanced flow control to find private methods will never be implemented, so mangle-regex is all we have.

@axkibe
Copy link

axkibe commented Jun 20, 2016

I agree, as posted before, I'm fiddling with Uglifyies internals to decide on a predefined list on to be mangled names and those not to be touched. Would be nice if this feature would have a more defined/documented API, but yes otherwise I agree, Uglify will never make full flow control, this kind of automatic mangling would need a full transpiled language, possible but not within the scope of uglify.

@evs-chris
Copy link

FWIW, I had the same idea as @hakatashi and did a test implementation here. It walks each top-level scope collecting stats on all property accesses and identifiers in that scope, checks to see which ones would actually save a bit of space by being hoisted, and then replaces the accesses like document.createDocumentFragment with document[a] with a blob of replacement vars injected at the top of the scope e.g. var a='createDocumentFragment',b='createElement'.

Running that on some large-ish (~450k that uglified to ~210k before doing the hoist) code saved about 15% (~180k), and depending on the code, looks like that's a pretty decent average figure. The minified code stays roughly as gzippable (60k) as it was before, so there's no real improvement on that front. A bit of performance testing seems to indicate a small performance penalty for doing the hoisting on Chrome. I haven't done any extensive testing on any other browsers.

I looked into implementing the transform on uglify originally, but the code is a bit over my head 😄

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

No branches or pull requests