1. #1
    Ext User OlleJonsson's Avatar
    Join Date
    May 2008
    Location
    Malmö, Sweden
    Posts
    30
    Vote Rating
    1
    OlleJonsson is on a distinguished road

      0  

    Post [PATCH] Minify-friendlier ExtJS

    [PATCH] Minify-friendlier ExtJS


    Dear Exters,

    I took the liberty of removing most jslint warnings from ExtJS' ext-all-debug.js file from the 2.2.1 distribution, just to see if it was impossible. Not impossible at all, it turned out.

    https://gist.github.com/raw/8936af07...t-all-debug.js

    JavaScript Lint now only reports these five warnings (all of which are harmless enough and won't break your minification -- they're just empty statements that don't need to be there):

    0 error(s), 5 warning(s)
    8410 WARNING: empty statement or extra semicolon
    } else {
    ........^
    8426 WARNING: empty statement or extra semicolon
    }
    ............^
    8932 WARNING: empty statement or extra semicolon
    }
    ............^
    9387 WARNING: empty statement or extra semicolon
    }
    ............^
    9421 WARNING: empty statement or extra semicolon
    } else {
    ................^

    What to do about these is more up to the code's original author. Some logic could be switched, to drop all of these.

    best regards,
    Olle

    Thoughts: Are there TextMate users in the ExtJS developer team? Perhaps they should drop the if(){}; and for(){}; constructs from its default JavaScript bundle, since these throw jslint errors. And, install the JavaScript Tools bundle:

    http://www.456bereastreet.com/archiv..._for_textmate/
    Attached Files

  2. #2
    Sencha - Ext JS Dev Team evant's Avatar
    Join Date
    Apr 2007
    Location
    Sydney, Australia
    Posts
    17,159
    Vote Rating
    674
    evant has a reputation beyond repute evant has a reputation beyond repute evant has a reputation beyond repute evant has a reputation beyond repute evant has a reputation beyond repute evant has a reputation beyond repute evant has a reputation beyond repute evant has a reputation beyond repute evant has a reputation beyond repute evant has a reputation beyond repute evant has a reputation beyond repute

      0  

    Default


    Thanks for the post, I'll check these out. As you said, most of them are harmless but it would be good to clean them up.
    Evan Trimboli
    Sencha Developer
    Twitter - @evantrimboli
    Don't be afraid of the source code!

  3. #3
    Sencha - Ext JS Dev Team Animal's Avatar
    Join Date
    Mar 2007
    Location
    Notts/Redwood City
    Posts
    30,546
    Vote Rating
    64
    Animal is a jewel in the rough Animal is a jewel in the rough Animal is a jewel in the rough

      0  

    Default


    Which original source modules need fixing though?

  4. #4
    Ext User OlleJonsson's Avatar
    Join Date
    May 2008
    Location
    Malmö, Sweden
    Posts
    30
    Vote Rating
    1
    OlleJonsson is on a distinguished road

      0  

    Default Juicer complains further

    Juicer complains further


    LOL. More work. Juicer was useful in complaining more. Juicer has strict settings by default, stricter than the laxer ones I had in my JavaScript Tools settings.

    What follows here is tip of an ice-berg of warnings. "Lint at line 967 character 28: Too many errors. (2% scanned)." I guess most of the structural style issues that hinder minification are in the above patch.

    Code:
    ollebox:extjs olle$ juicer verify ext-all-debug.js 
    Verifying ext-all-debug.js with JsLint
      Problems detected
      Lint at line 34 character 27: ['style'] is better written in dot notation.
      var s = o["style"];
      Lint at line 51 character 41: ['cls'] is better written in dot notation.
      b += ' class="' + o["cls"] + '"';
      Lint at line 53 character 39: ['htmlFor'] is better written in dot notation.
      b += ' for="' + o["htmlFor"] + '"';
      Lint at line 31 character 9: The body of a for in should be wrapped in an if statement to filter unwanted properties from the prototype.
      for(var attr in o){
      Lint at line 91 character 38: ['cls'] is better written in dot notation.
      el.className = o["cls"];
      Lint at line 88 character 13: The body of a for in should be wrapped in an if statement to filter unwanted properties from the prototype.
      for(var attr in o){
      Lint at line 208 character 51: Use '!==' to compare with 'null'.
      while ((matches = re.exec(styles)) != null){
      Lint at line 212 character 16: The body of a for in should be wrapped in an if statement to filter unwanted properties from the prototype.
      for (var style in styles){
      Lint at line 227 character 23: Expected a conditional expression and instead saw an assignment.
      if(rs = insertIntoTable(el.tagName.toLowerCase(), where, el, html)){
      Lint at line 264 character 17: Expected a 'break' statement before 'case'.
      }
      Lint at line 274 character 17: Expected a 'break' statement before 'case'.
      }
      Lint at line 413 character 17: Unescaped '-'.
      re : /\{([\w-]+)(?:\:([\w\.]*)(?:\((.*?)?\))?)?\}/g,
      Lint at line 446 character 9: eval is evil.
      eval(body);
      Lint at line 446 character 9: eval is evil.
      eval(body);
      Lint at line 553 character 31: Expected a conditional expression and instead saw an assignment.
      for(var i = 0, ci; ci = c[i]; i++){
      Lint at line 588 character 35: Expected a conditional expression and instead saw an assignment.
      for(var i = 0, ni; ni = ns[i]; i++){
      Lint at line 590 character 39: Expected a conditional expression and instead saw an assignment.
      for(var j = 0, ci; ci = cs[j]; j++){
      Lint at line 596 character 23: 'i' is already defined.
      for(var i = 0, ni, cn; ni = ns[i]; i++){
      Lint at line 596 character 30: 'ni' is already defined.
      for(var i = 0, ni, cn; ni = ns[i]; i++){
      Lint at line 596 character 39: Expected a conditional expression and instead saw an assignment.
      for(var i = 0, ni, cn; ni = ns[i]; i++){
      Lint at line 598 character 27: 'j' is already defined.
      for(var j = 0, cj; cj = cn[j]; j++){
      Lint at line 598 character 39: Expected a conditional expression and instead saw an assignment.
      for(var j = 0, cj; cj = cn[j]; j++){
      Lint at line 605 character 22: 'utag' is already defined.
      var utag = tagName.toUpperCase();
      Lint at line 606 character 23: 'i' is already defined.
      for(var i = 0, n; n = ns[i]; i++){
      Lint at line 606 character 33: Expected a conditional expression and instead saw an assignment.
      for(var i = 0, n; n = ns[i]; i++){
      Lint at line 613 character 23: 'i' is already defined.
      for(var i = 0, n; n = ns[i]; i++){
      Lint at line 613 character 29: 'n' is already defined.
      for(var i = 0, n; n = ns[i]; i++){
      Lint at line 613 character 33: Expected a conditional expression and instead saw an assignment.
      for(var i = 0, n; n = ns[i]; i++){
      Lint at line 642 character 31: Expected a conditional expression and instead saw an assignment.
      for(var i = 0, ci; ci = cs[i]; i++){
      Lint at line 658 character 30: Expected a conditional expression and instead saw an assignment.
      for(var i = 0,ci; ci = cs[i]; i++){
      Lint at line 670 character 31: Expected a conditional expression and instead saw an assignment.
      for(var i = 0, ci; ci = cs[i]; i++){
      Lint at line 702 character 5: eval is evil.
      eval("var batch = 30803;");
      Lint at line 702 character 5: eval is evil.
      eval("var batch = 30803;");
      Lint at line 717 character 19: 'i' is already defined.
      for(var i = 0, len = cs.length; i < len; i++){
      Lint at line 717 character 28: 'len' is already defined.
      for(var i = 0, len = cs.length; i < len; i++){
      Lint at line 736 character 22: Expected a conditional expression and instead saw an assignment.
      for(i = 1; c = cs[i]; i++){
      Lint at line 744 character 33: Expected a conditional expression and instead saw an assignment.
      for(j = i+1; cj = cs[j]; j++){
      Lint at line 762 character 19: 'i' is already defined.
      for(var i = 0, len = c2.length; i < len; i++){
      Lint at line 762 character 28: 'len' is already defined.
      for(var i = 0, len = c2.length; i < len; i++){
      Lint at line 767 character 19: 'i' is already defined.
      for(var i = 0, len = c1.length; i < len; i++){
      Lint at line 767 character 28: 'len' is already defined.
      for(var i = 0, len = c1.length; i < len; i++){
      Lint at line 786 character 19: 'i' is already defined.
      for(var i = 0, len = c2.length; i < len; i++){
      Lint at line 860 character 50: Be careful when making functions within a loop. Consider putting the function in a closure.
      });
      Lint at line 877 character 13: eval is evil.
      eval(fn.join(""));
      Lint at line 877 character 13: eval is evil.
      eval(fn.join(""));
      Lint at line 955 character 29: Unescaped '-'.
      re: /^\.([\w-]+)/,
      Lint at line 958 character 29: Unescaped '-'.
      re: /^\:([\w-]+)(?:\(((?:[^\s>\/]*|.*?))\))?/,
      Lint at line 961 character 44: Unescaped '-'.
      re: /^(?:([\[\{])(?:@)?([\w-]+)\s?(?:(=|.=)\s?['"]?(.*?)["']?)?[\]\}])/,
      Lint at line 964 character 28: Unescaped '-'.
      re: /^#([\w-]+)/,
      Lint at line 967 character 28: Unescaped '-'.
      re: /^@([\w-]+)/,
      Lint at line 967 character 28: Too many errors. (2% scanned).

  5. #5
    Sencha - Ext JS Dev Team Animal's Avatar
    Join Date
    Mar 2007
    Location
    Notts/Redwood City
    Posts
    30,546
    Vote Rating
    64
    Animal is a jewel in the rough Animal is a jewel in the rough Animal is a jewel in the rough

      0  

    Default


    It's not a huge help fixing things in ext-all-debug.

    That is built from individual source modules, so which sources need to be fixed?

    And really, "eval is evil" is getting a little tired now.

    The authors of that tool might consider that "eval is usually not necessary" is less hyperbolic.

  6. #6
    Ext User OlleJonsson's Avatar
    Join Date
    May 2008
    Location
    Malmö, Sweden
    Posts
    30
    Vote Rating
    1
    OlleJonsson is on a distinguished road

      0  

    Default


    I could make a long list, which would take some time. Fixing it in ext-all-debug.js was mostly to see if it was possible to quiet warnings.

    Code:
    jquery-bridge.js
    core/
        CompositeElement.js
        DomHelper.js
        DomQuery.js
        Element.js
        EventManager.js
        Ext.js
    Those are the first files that had warnings for structure.

    @Animal: About the rules in jslint: Yes, I don't use the default settings either, it's just that default tool settings can matter. If you go around them, it's useful to develop settings files, or publishing decisions like "We don't use the following rules."

    I have not paid for access to the SVN repository, so I have some questions about workflow. Apologies if these are FAQs, just direct me to an answer if that's the case.

    Would it be OK license-wise for me to import a 2.2.1 dist to GitHub or Bitbucket and work further there? That would simplify for me as a contributor. (If I can't use Bitbucket, I could just use a local Mecurial repo, and send you diffs against it.)
    Last edited by OlleJonsson; 23 Mar 2009 at 4:39 AM. Reason: added local Mercurial idea

  7. #7
    Sencha - Ext JS Dev Team Animal's Avatar
    Join Date
    Mar 2007
    Location
    Notts/Redwood City
    Posts
    30,546
    Vote Rating
    64
    Animal is a jewel in the rough Animal is a jewel in the rough Animal is a jewel in the rough

      0  

    Default


    It could be a bit difficult for you to contribute changes.

    Because the code has moved on significantly in SVN from the 2.2.1 tagged revision.

    And there is also the 3.0 branch in SVN.

  8. #8
    Ext User OlleJonsson's Avatar
    Join Date
    May 2008
    Location
    Malmö, Sweden
    Posts
    30
    Vote Rating
    1
    OlleJonsson is on a distinguished road

      0  

    Default


    Idea: Have your legal dept. make it possible for "code gnomes" like me to have read-only anonymous access to the SVN trunk (or the 2.2.x branch) so that the maintenance burden could be distributed throughout the community. Free help. I guess the documentation spelling errors and examples could get brushed up via patches, too.

    (Edit: Now I see that to contribute current patches, I should currently pay $999 per annum. Hard to justify to my project manager - "I want to use company time to fix a detail in a library we pay for, and I need to pay for helping.") I'm not bitter, I'm just saying this is a "blocker" hurdle for me.

  9. #9
    Sencha - Ext JS Dev Team Animal's Avatar
    Join Date
    Mar 2007
    Location
    Notts/Redwood City
    Posts
    30,546
    Vote Rating
    64
    Animal is a jewel in the rough Animal is a jewel in the rough Animal is a jewel in the rough

      0  

    Default


    Do remember that I'm not part of the Ext company, just a volunteer moderator and documenter.

    I agree that there could be some kind of community contribution. But I don't know how it could be managed.

    It can't be a free-for-all because Ext's code is finely designed, authored and tuned. And it's a commercial project, so you can't have dozens, or perhaps hundreds of amateur enthusiasts, each taking the code in the direction they think it should go...

    I don't know what the answer is.

    I and most others contribute code through the Feature Requests folder, or the Bug Reports folder.

Thread Participants: 2