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

Use a minimal function declaration as the token string to replace preserved comment block #17

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

leik
Copy link

@leik leik commented Aug 19, 2013

When a preserved comment block is followed by an IIFE, the minified js doesn't pass syntax parsing, like closure compiler, because of missing brackets around the IIFE in minified js file.

For example:

/*! important comment */

/**
 * @module Test
 */
(function() {
    console.log('test');
}());

The snippet above is a minimal version of a yui 2 module, when wrapped as a yui2in3 module, including YUI2 variable and version information at the bottom, minified version could be:

YUI.add("yui2-Test",function(e,t){var n=e.YUI2;
/*! important comment */
function(){console.log("test")}()},"@VERSION@");

However, the minified result doesn't compile in closure compiler , neither does it in any browser.
Errors in closure compiler:
JSC_PARSE_ERROR: Parse error. syntax error at line 4 character 32
function(){console.log("test")}()},"@Version@");
^
JSC_PARSE_ERROR: Parse error. missing } after function body at line 4 character 47
function(){console.log("test")}()},"@Version@");
^
The missing brackets around the IIFE is the cause.

The deeper reason behind this is that yuglify uses a simple string as the token to replace the preserved comment, i.e.:

token = '"yUglify: preserved comment block"'

If preserved comment block is followed by an IIFE, after uglify minifies the js file, in which preserved comments has been replaced with tokens, it adds a comma between the token string and the IIFE and removes the surrounding brackets of the IIFE, because uglify joins consecutive simple statements into sequences using the “comma operator”. (if I understand correctly after a deep debugging.)

Therefore instead of using a simple string as the token and handles the inconsistency that sometimes it adds comma sometimes it doesn't, a minimal function could be a better alternative:

token = 'function fn(){"yUglify: preserved comment block";}'

and the uglify result is consistent for all the cases. And of course it fixes the IIFE after preserved comment block:

YUI.add("yui2-Test",function(e,t){var n=e.YUI2;
/*! important comment */
(function(){console.log("test")})()},"@VERSION@");

It requires a minor fix of shifter's tests, which requires another PR yui/shifter#97.

… that replaces all the copyright comments.
@leik
Copy link
Author

leik commented Aug 21, 2013

With this pull request change, we just discovered that if there are any global function ( function foo() { ... } ) declarations in the raw file, in the output file they will be pulled onto the top and potentially before the preserved comment block. But a workaround is to use function assignment instead (var foo = function() {...} or window.foo = function() {...}).

This has been fixed by leik@bc7bef9

@rgrove
Copy link

rgrove commented Aug 28, 2013

Alternatively, yuglify could upgrade to Uglify v2, which has internal support for preserving comments and can easily be configured to support YUI's comment preservation format.

…) { ... } ) declarations in the raw file, in the output file they will be pulled onto the top and potentially before the preserved comment block.
leik pushed a commit to leik/shifter that referenced this pull request Feb 26, 2014
leik pushed a commit to leik/shifter that referenced this pull request Feb 26, 2014
@leik
Copy link
Author

leik commented Mar 19, 2014

Hi guys, can anybody come back and have a look at this PR please? I evolved the PR so that it now works for all possible cases and doesn't need any workaround. And It requires a minor fix of shifter's tests, which requires another PR yui/shifter#97.

@erkiesken
Copy link

Just FYI I was bitten by this issue just now as well. I was trying to wrap Hammer.js library in a YUI3 custom module. Something like this:

YUI.add("my-hammer", function (Y) {
// Following is inserted by grunt script
/*! Hammer.JS - v1.1.3 - 2014-06-12
 * http://eightmedia.github.io/hammer.js
 *
 * Copyright (c) 2014 Jorik Tangelder <[email protected]>;
 * Licensed under the MIT license */
(function(window, undefined) {
   // ...etc
})(window);

}, "0.0.1", {requires: []});

And the parentheses around their function gets removed and results in a JS syntax error.

Best fix probably is to upgrade yuglify to UglifyJS 2.x as per #7.

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.

3 participants