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

option for not dropping function expression names #552

Closed
getify opened this issue Sep 17, 2014 · 19 comments
Closed

option for not dropping function expression names #552

getify opened this issue Sep 17, 2014 · 19 comments

Comments

@getify
Copy link

getify commented Sep 17, 2014

There are reasons other than lexical reference why people may rely on names of named function expressions.

See:

Please add an option that preserves all function expression names.

Note: It is unclear if such an option should continue to allow such (preserved) function expression names to be mangled or not. On one hand, it would be nice if they wouldn't be mangled, because that could make debug stack traces (even in production code) much more useful. OTOH, function declaration names would still be mangled, so parts of the debug stack traces (in production code) would still be much less useful.

Perhaps then mangling should still happen on the preserved function expression names, but then another option could say "don't mangle any function names"?

@amireh
Copy link

amireh commented Sep 25, 2014

Hello, I need to +1 this because this has bitten me today and I can no longer deploy any updates to the app.

I have a genuine use for keeping (and referencing) function names. Your argument that they should not be relied upon (in one of the relevant threads) does not really solve my problem. Function.prototype.name is everywhere to be found past IE8.

Can you provide us with an option to either keep all function expression names, or at least get to explicitly specify the names of the functions we want to keep (they are static, after-all)?

@monsanto
Copy link

+1 this screws up regenerator

@sebmck
Copy link

sebmck commented Dec 12, 2014

@monsanto How does this screw up regenerator?

@jlongster
Copy link

@monsanto How does this screw up regenerator?

The runtime has this: var GF = function GeneratorFunction() {}; and later in the code it uses GF.name. Uglify currently drops the name completely. Please don't drop them!

@rvanvelzen
Copy link
Collaborator

You can pass -c "unused=false" to the compressor, which will prevent it from dropping otherwise-unused names.

You can simply not pass -m to prevent mangling altogether.

Is there any issue here that is not solved by either of those options?

@getify
Copy link
Author

getify commented Jan 4, 2015

You can pass -c "unused=false"

Aren't there other places besides function expression names that this would affect? For example, unused parameters, unused var declarations, etc. This request is specifically for an option to control function expressions only.

You can simply not pass -m to prevent mangling altogether.

Mangling is critical to reducing file size. Turning it completely off is a total non-starter. Again, this issue is requesting control over the mangling ONLY of function expression names, as those are typically included specifically for debugging purposes.

@rvanvelzen
Copy link
Collaborator

Thanks for your clarification.

  • Adding an option to prevent dropping of function expression names is entirely feasible
  • Not mangling function expression names is probably trickier, but I haven't looked into it but I could be wrong.

Would -c "keep_fnames" suffice for the first point? Would you prefer a different option name?

@getify
Copy link
Author

getify commented Jan 4, 2015

I won't bikeshed on naming of the options, and will leave that to your expertise, but "keep_fnames" sounds pretty fine to me.

One further clarification, from my OP in the thread... the desire on suppressing mangling would be to not mangle any function names, regardless of function expression vs function decl. The reason is for stack traces, where all function names are useful to understand context of an error.

IOW, we'd not care nearly as much about variable names having been mangled (good for smaller file size!) but function names have much more importance in stack traces so an option to keep them as-is would be very nice. :)

@getify
Copy link
Author

getify commented Jan 4, 2015

Furthermore, it might be that these two different requests logically belong together, combined as a single option, perhaps called "keep_fnames". Or it might be better to have them separately choosable for greater flexibility.

I don't have a strong opinion either way on that. I can say that my likely use would be to always want either both or neither, so having a combined option that implied both wouldn't restrict my use cases. But that's just one anecdote to consider.

rvanvelzen added a commit to rvanvelzen/UglifyJS2 that referenced this issue Jan 4, 2015
…ssion names

See mishoo#552. This is useful for stack traces.
@rvanvelzen
Copy link
Collaborator

I think that having separate options make slightly more sense. Among other things, it's easier for testing.

Retaining function expression names is implemented in #602. It should do the trick.

About the function name mangling: I think you're probably right in that it should be all function names. It doesn't really make sense to mix mangled and non-mangled function names.

My idea is to have a flag for mangling, which would give: -m "no_funcs", which would then stop function names from being mangled.

rvanvelzen added a commit to rvanvelzen/UglifyJS2 that referenced this issue Jan 4, 2015
See mishoo#552. This is mostly useful for having the actual function names in traces.
rvanvelzen added a commit to rvanvelzen/UglifyJS2 that referenced this issue Jan 4, 2015
See mishoo#552. This is mostly useful for having the actual function names in traces.
@getify
Copy link
Author

getify commented Jan 4, 2015

<3 thanks!!

mishoo added a commit that referenced this issue Jan 5, 2015
Passing `--keep-fnames` will enable it both for compressor/mangler, so that
function names will not be dropped (when unused) nor mangled.
rvanvelzen added a commit to rvanvelzen/UglifyJS2 that referenced this issue Jan 5, 2015
…ssion names

See mishoo#552. This is useful for stack traces.
rvanvelzen added a commit to rvanvelzen/UglifyJS2 that referenced this issue Jan 5, 2015
See mishoo#552. This is mostly useful for having the actual function names in traces.
rvanvelzen pushed a commit to rvanvelzen/UglifyJS2 that referenced this issue Jan 5, 2015
Passing `--keep-fnames` will enable it both for compressor/mangler, so that
function names will not be dropped (when unused) nor mangled.
@rvanvelzen
Copy link
Collaborator

I'll close this now, as the functionality now exists (--keep-fargs as a global CLI option, and seperately for -c and -m as well). It will be included in the next release.

Thanks!

@getify
Copy link
Author

getify commented Jan 6, 2015

Sorry, what does --keep-fargs option relate to? the function name mangling, or the function name removal?

@mishoo
Copy link
Owner

mishoo commented Jan 6, 2015

--keep-fargs just prevents removal of unused function arguments. Even when not removed, they still get mangled.

--keep-fnames prevents both removal and mangling of function names. That's useful for those who rely on Function.prototype.name.

@rvanvelzen
Copy link
Collaborator

Whoops, sorry. Mistyped there - I meant --keep-fnames.

@getify
Copy link
Author

getify commented Jan 6, 2015

OK, so I thought there were going to be two separate options, one for not dropping function names and one for not mangling them? Perhaps I misunderstood, or did those two options just get combined into --keep-fnames?

@rvanvelzen
Copy link
Collaborator

For both the compressor and mangler there are separate options:

  • -c "keep_fnames" will prevent function names from being dropped
  • -m "keep_fnames" will prevent function names from being mangled

As a shorthard, --keep-fnames enables both these options. I hope that satisfies your question. :-)

@mishoo
Copy link
Owner

mishoo commented Jan 6, 2015

More exactly, compressor options are different from mangler options. In this specific case, both accept a boolean option with the name keep_fnames. If this option is passed to the compressor, the function names will not be dropped (even if unused is true, which is the default). If it's also passed to the mangler, the function names will not be modified.

As @rvanvelzen stated, there's also a “global” option which can be passed to the command-line tool, --keep-fnames, which will do what hopefully everyone expect, that is, pass both those options to the compressor and mangler and generate code safe for the poor souls relying on Function.prototype.name at run-time.

Hope this clarifies.

@getify
Copy link
Author

getify commented Jan 6, 2015

awesomeness all around. thanks so much for the clarifications, you all rock!

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

7 participants