rspivak / slimit

SlimIt - a JavaScript minifier/parser in Python
MIT License
551 stars 94 forks source link

Mangling and top level symbols #27

Closed davidkhess closed 12 years ago

davidkhess commented 12 years ago

Some of the javascript on my page is loaded from elsewhere and the rest I'm running though slimit with mangle=True. Unfortunately, this doesn't work for me because slimit is mangling global symbols which I need to remain unmangled (i.e. module identifiers).

I came up with the following patch which seemed to do the trick for me:

diff --git a/src/slimit/visitors/scopevisitor.py b/src/slimit/visitors/scopevisitor.py
index 56585dc..7e0a2fd 100644
--- a/src/slimit/visitors/scopevisitor.py
+++ b/src/slimit/visitors/scopevisitor.py
@@ -145,12 +145,13 @@ def mangle_scope_tree(root):
             scope.mangled[name] = mangled_name
             scope.rev_mangled[mangled_name] = name

-    def visit(node):
-        mangle(node)
+    def visit(node, mangle_this_node=True):
+        if mangle_this_node:
+            mangle(node)
         for child in node.children:
             visit(child)

-    visit(root)
+    visit(root, mangle_this_node=False)

 def fill_scope_references(tree):

I have no idea if that is the appropriate way to handle this but it worked for me.

davidkhess commented 12 years ago

After thinking about it some more, the core of the problem was that I was losing a reference to $ (jQuery) – I believe the mangler was using $ for a symbol of it's own. So, it may be more of an issue that somehow the mangler needs to know what identifiers it should not use when mangling at the top level.

Regardless, of how you look at it, this approach still handles it by leaving the global scope alone.

rspivak commented 12 years ago

I think it's a good idea not to mess with global scope by default. I made some changes so that Slimit doesn't mangle global scope any more and added a -t / --mangle-toplevel command line option that allows you to enable the global scope mangling for better minification results if that's what one needs and understands the possible consequences.

davidkhess commented 12 years ago

Sounds like a good solution. Thanks!