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

fix negate_iife : false #2

Closed
wants to merge 1 commit into from
Closed

Conversation

tomByrer
Copy link
Contributor

Defaults will scramble IIFEs & program completion values, which affect the majority JS I feed into this tool. (eg any jQuery plugs)

Defaults will scramble [IIFEs & program completion values](mishoo/UglifyJS#543), which affect the majority JS I feed into this tool.
@Skalman
Copy link
Owner

Skalman commented Sep 28, 2014

If I understand correctly, the only use case for this is in bookmarklets, which doesn't seem like that big of a use case.

Would resolving #3 alleviate the need to change this default?

@tomByrer
Copy link
Contributor Author

The larger issue is people breaking the code who don't know said code very well (eg myself minimizing 3rd party code). IMHO if people were minimizing their own code that they are working on, then they would use a build tool locally.
Ugify-JS should never had put this as default.

@tomByrer
Copy link
Contributor Author

Would resolving #3 alleviate the need to change this default?

Not really; I'll still need to verify if the flag is still flipped. I re-install browsers & OS alot (testing).

@Skalman
Copy link
Owner

Skalman commented Sep 29, 2014

If the problem is the possibility of breaking other's code in general, unsafe and unsafe_comps should also be disabled by default. I'm not convinced that we should disable useful optimizations by default.

I might consider creating different modes, though. Perhaps one "normal" and one completely "safe".

@tomByrer
Copy link
Contributor Author

On jsDelivr, I likely have thousands of people using unsafe compression via your webpage. Maybe even 1000s/day. I haven't heard any complaints.
This is why my scope is limited to only 1 config that does remove IIFEs

@Skalman
Copy link
Owner

Skalman commented Sep 30, 2014

Is the complaint that it doesn't work in bookmarklets, or in which other situations can this be a problem?

@tomByrer
Copy link
Contributor Author

I don't use this as a bookmarklet; I visit directly https://skalman.github.io/UglifyJS-online/ & don't always remember to check the options tab/button. Other defaults are fine & don't seem destructive.

@Skalman
Copy link
Owner

Skalman commented Sep 30, 2014

I think you misunderstood what I was saying.
The scripts that you're uglifying, are they bookmarklets? Because I don't know of any other situation where the program completion value matters.

@tomByrer
Copy link
Contributor Author

Sorry, no, 70% of my use-case for UglifyJS-online is to minify other peoples' jQuery for [CDN hosting]. I haven't tested the actual output, but I don't have that time & I can see the IIFE is mangled.
Another 25% is non-jQuery for jsDelivr. which don't use IIFE as much as jQuery.

@Skalman
Copy link
Owner

Skalman commented Oct 3, 2014

I don't want to merge this without understanding the benefit. Could you post or link to a script that somebody has complained about?
The only situations where the program completion value matters, that I can think of, are

  1. Bookmarklets
  2. HTML event attribute values (e.g. onclick="")
  3. Input into eval() or new Function()

@tomByrer
Copy link
Contributor Author

tomByrer commented Oct 4, 2014

source:

/*!
 * jQuery JavaScript Library v2.1.1...
 * Date: 2014-05-01T17:11Z
 */
(function( global, factory ) {
    if ( typeof module === "object" && typeof module.exports === "object" ) {
        module.exports = global.document ?
            factory( global, true ) :
            function( w ) {
                if ( !w.document ) {
                    throw new Error( "jQuery requires a window with a document" );
                }
                return factory( w );

~~~snip~~~

jQuery.noConflict = function( deep ) {
    if ( window.$ === jQuery ) {
        window.$ = _$;
    }
    if ( deep && window.jQuery === jQuery ) {
        window.jQuery = _jQuery;
    }
    return jQuery;
};
if ( typeof noGlobal === strundefined ) {
    window.jQuery = window.$ = jQuery;
}
return jQuery;
}));

After http://skalman.github.io/UglifyJS-online/ with negate_iife : true

!function(e,t){"object"==typeof module&&"object"==typeof module.exports?module.exports=e.document?t(e,!0):function(e){if(!e.document)throw Error("jQuery requires a...
~~~snip~~~
Z.noConflict=function(t){return e.$===Z&&(e.$=Wn),t&&e.jQuery===Z&&(e.jQuery=Rn),Z},typeof t===kt&&(e.jQuery=e.$=Z),Z});

Maybe it doesn't matter, but visually (function( global, factory ) { is not the same as !function(global,factory){. Also the last line seem different.

Do as you wish, I have nothing more to add to this matter.

@Skalman
Copy link
Owner

Skalman commented Oct 5, 2014

So do I understand correctly that the only problem is that it looks different, and that there is no functional change whatsoever?
I will only change settings that improve either functionality (e.g. fix a bug) or compression. The goal of minification is compression, not readability.

@tomByrer
Copy link
Contributor Author

tomByrer commented Oct 5, 2014

I don't know; I don't have time to test every JS I want minifed.
& I don't have any more time to waste on this thread; spent 30 minutes of posts to change 5 letters.

@tomByrer tomByrer closed this Oct 5, 2014
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 this pull request may close these issues.

2 participants