google / closure-compiler

A JavaScript checker and optimizer.
https://developers.google.com/closure/compiler/
Apache License 2.0
7.35k stars 1.14k forks source link

Reinstate --variable_map_output_file #433

Closed cugh closed 10 years ago

cugh commented 10 years ago

The --variable_map_output_file command line option has been dropped in v20140508.

This was a very useful option. I'm getting bug reports with obfuscated stack trace all the time and I used the variable map to quickly figure out what went wrong. The variable map was very easy to use because the original and obfuscated name are on the same line.

The source map file is not useful at all, as it is nearly impossible to open it in an editor and manually figure out quickly what is the original name of an obfuscated variable.

Could you please reinstate the --variable_map_output_file?

nicks commented 10 years ago

Source maps have a semi-formal specification and broad tooling support.

Variable/Property output maps were a format we played with before source maps, and had lots of problems. I don't think we're going to support an idiosyncratic second debugging format with poor support.

my own project has an admin console where you paste an obfuscated stack trace, and get back the unobfuscated version. It is simple with off-the-shelf tools, like https://github.com/mozilla/source-map/

ChadKillingsworth commented 10 years ago

Source and property maps also had a couple of other undocumented uses: http://stackoverflow.com/questions/19491592/changing-javascript-variable-and-function-name-from-java-using-rhino-or-closure/19567004#19567004

However these are not officially supported either. If sourcemaps is the official direction, then I think they should only be available from the Java API or custom builds.

concavelenz commented 10 years ago

I think it is reasonable to keep the output files. Until browser provide accurate column information (which only later IEs and Chrome provide currently) the var maps are useful. I think the primary problem we had was the misuse of the maps as inputs to provide cross-compilation linking. Which is not what we want to try to support.

cugh commented 10 years ago

I know there are specialized tools to work with source maps. However, our typical scenario is as follows: A developer who doesn't know JavaScript very well has to quickly figure out what's wrong while knowing only obfuscated function name, e.g. 'ofn'.

With variable maps, such developer could quickly figure out to run 'grep ofn var.map' and immediately know where the problem lies. It was great, because:

  1. Non-web devs with very little JavaScript skills could figure out quickly themselveshow to do it.
  2. Didn't require any specialized tools. It's a pain i.t.a. to maintain such tool on all testing virtual machines, and almost impossible at clients' machines where we sometimes need to work to quickly debug and fix things.
  3. One doesn't need to research what source maps are, what tools to use. (Again think about a dev who has never heard of source maps.) We even have situations where something has to be fixed in top-security environment (not connected to the Internet) and then trying to use the source map for the first time is hopeless.
  4. In fact variable maps were so simple, that even grepping wasn't necessary, just opening them in any editor did the job.

Any good format should be human-readable and processable with general tools. Source maps aren't.

cugh commented 10 years ago

Hmm... I can see in the new --help that there seems to be no way of preserving name mappings between compilations. I used to use variable and property maps for it (--variable_map_input_file), but there is no equivalent for source maps.

It was very useful to e.g. preserve the minification of 'myFunction1' as 'abc' always. Without this as I add other functions, now they could suddenly get minified to 'abc', and 'myFunction1' to something else.

This can complicate bug fixing, as any error report needs to come with a guaranteed exact version number (yes, sometimes 'ad.sf is undefined' is the entire report I get). And then one needs to retrieve old source map corresponding to exactly this version instead of just simply always using the current one. This of course also means a very strict source control policy among all devs (forgetting to commit a source map together with JavaScript could lead to a disaster where a debugging person would use a wrong source map version).

So for persistent minification, the variable and source maps were the only option, and I'd love to see them undeleted.

nicks commented 10 years ago

My main opposition to variable/property maps is that they are buggy, unsupported by other tools, and have a maintenance cost on the overall compiler.

OP's last comment scares me even more, because it suggests that they've been building critical processes on top of these unmaintained tools. That seems like a recipe for disaster, and exactly the situation we're hoping to avoid by removing them.

ChadKillingsworth commented 10 years ago

My opinion is that the output files don't necessarily cause an issue. Perhaps however they should be labeled as a renaming report. Source maps have known problems which have been listed. I share Nick's concern on the input side of things however. We haven't officially supported directed renaming and removing the input files was the correct call. A custom build of the compiler isn't too big of a hurdle to enable that option.

concavelenz commented 10 years ago

I like renaming it a variable renaming report.

cugh commented 10 years ago

Could the devs please vote on it and decide what will be the destiny for all four options, just so the issue doesn't stagnate: --variable_map_input_file --variable_map_output_file --property_map_input_file --property_map_output_file

As a user, I would love to keep all 4 options, because I depend on them. Preferably as command line options, and if that's impossible, then as something callable from Java API. Renaming them to something else is perfectly fine.

nicks commented 10 years ago

to clarify, we'll likely discuss this at monday's meeting. This is not a democracy; @concavelenz's vote probably counts for more because the technical costs will likely fall on him (though where the burden falls is one of the things we'll discuss)