Browse Source

Add a new rule for Synchronous XMLHttpRequest (#178)

* Fix bug in spying function
* New rule Synchronous XHR
* Add the synchronousXHR rule to the dashboard
Gaël Métais 9 years ago
parent
commit
e1316ea48b

+ 1 - 1
front/src/views/rule.html

@@ -155,7 +155,7 @@
                         (<ng-pluralize count="offender.requests" when="{'one':'1 request','other':'{} requests'}"></ng-pluralize>)
                         (<ng-pluralize count="offender.requests" when="{'one':'1 request','other':'{} requests'}"></ng-pluralize>)
                     </div>
                     </div>
 
 
-                    <div ng-if="policyName === 'globalVariables' || policyName === 'jQueryVersionsLoaded'">
+                    <div ng-if="policyName === 'globalVariables' || policyName === 'jQueryVersionsLoaded' || policyName === 'synchronousXHR'">
                         {{offender}}
                         {{offender}}
                     </div>
                     </div>
 
 

+ 9 - 0
lib/metadata/policies.js

@@ -241,6 +241,15 @@ var policies = {
             };
             };
         }
         }
     },
     },
+    "synchronousXHR": {
+        "tool": "phantomas",
+        "label": "Synchronous Ajax requests",
+        "message": "<p>Making an XMLHttpRequest with the <i>async</i> option set to <i>false</i> is deprecated due to the negative effect to performances. The browser's main thread needs to stop everything until the response is received.</p>",
+        "isOkThreshold": 0,
+        "isBadThreshold": 1,
+        "isAbnormalThreshold": 1,
+        "hasOffenders": true
+    },
     "consoleMessages": {
     "consoleMessages": {
         "tool": "phantomas",
         "tool": "phantomas",
         "label": "Console messages",
         "label": "Console messages",

+ 2 - 1
lib/metadata/scoreProfileGeneric.json

@@ -50,6 +50,7 @@
             "policies": {
             "policies": {
                 "jsErrors": 1,
                 "jsErrors": 1,
                 "documentWriteCalls": 2,
                 "documentWriteCalls": 2,
+                "synchronousXHR": 5,
                 "consoleMessages": 0.5,
                 "consoleMessages": 0.5,
                 "globalVariables": 0.5
                 "globalVariables": 0.5
             }
             }
@@ -108,7 +109,7 @@
         "domComplexity": 2,
         "domComplexity": 2,
         "domManipulations": 2,
         "domManipulations": 2,
         "scroll": 1,
         "scroll": 1,
-        "badJavascript": 1,
+        "badJavascript": 2,
         "jQuery": 1,
         "jQuery": 1,
         "cssSyntaxError": 1,
         "cssSyntaxError": 1,
         "cssComplexity": 1,
         "cssComplexity": 1,

+ 3 - 2
lib/tools/phantomas/custom_modules/core/scopeYLT/scopeYLT.js

@@ -49,7 +49,7 @@ exports.module = function(phantomas) {
                             var err;
                             var err;
                             
                             
                             // Before
                             // Before
-                            if (enabled) {
+                            if (enabled && callbackBefore) {
                                 callbackBefore.apply(this, arguments);
                                 callbackBefore.apply(this, arguments);
                             }
                             }
 
 
@@ -71,7 +71,8 @@ exports.module = function(phantomas) {
 
 
                                 // After
                                 // After
                                 if (enabled && callbackAfter) {
                                 if (enabled && callbackAfter) {
-                                    callbackAfter.call(this, result, arguments);
+                                    var args = Array.prototype.slice.call(arguments);
+                                    callbackAfter.apply(this, [result].concat(args));
                                 }
                                 }
 
 
                                 if (err) {
                                 if (err) {

+ 30 - 0
lib/tools/phantomas/custom_modules/modules/ajaxReqYLT/ajaxReqYLT.js

@@ -0,0 +1,30 @@
+/**
+ * Analyzes AJAX requests
+ */
+/* global window: true */
+
+exports.version = '0.2.a';
+
+exports.module = function(phantomas) {
+    'use strict';
+
+    phantomas.setMetric('ajaxRequests'); // @desc number of AJAX requests
+    phantomas.setMetric('synchronousXHR'); // @desc number of synchronous 
+
+    phantomas.on('init', function() {
+        phantomas.evaluate(function() {
+            (function(phantomas) {
+                phantomas.spy(window.XMLHttpRequest.prototype, 'open', null, function(result, method, url, async) {
+                    phantomas.incrMetric('ajaxRequests');
+                    phantomas.addOffender('ajaxRequests', '<%s> [%s]', url, method);
+
+                    if (async === false) {
+                        phantomas.incrMetric('synchronousXHR');
+                        phantomas.addOffender('synchronousXHR', url);
+                        phantomas.log('ajaxRequests: synchronous XMLHttpRequest call to <%s>', url);
+                    }
+                }, true);
+            })(window.__phantomas);
+        });
+    });
+};

+ 5 - 4
lib/tools/phantomas/phantomasWrapper.js

@@ -39,18 +39,19 @@ var PhantomasWrapper = function() {
             'analyze-css': true,
             'analyze-css': true,
             'ignore-ssl-errors': true,
             'ignore-ssl-errors': true,
             'skip-modules': [
             'skip-modules': [
-                'domHiddenContent', // overriden
+                'ajaxRequests', // overridden
+                'domHiddenContent', // overridden
                 'domMutations', // not compatible with webkit
                 'domMutations', // not compatible with webkit
-                'domQueries', // overriden
+                'domQueries', // overridden
                 'events', // overridden
                 'events', // overridden
                 'filmStrip', // not needed
                 'filmStrip', // not needed
                 'har', // not needed for the moment
                 'har', // not needed for the moment
                 'javaScriptBottlenecks', // needs to be launched after custom module scopeYLT
                 'javaScriptBottlenecks', // needs to be launched after custom module scopeYLT
                 'jQuery', // overridden
                 'jQuery', // overridden
                 'jserrors', // overridden
                 'jserrors', // overridden
-                'lazyLoadableImages', //overriden
+                'lazyLoadableImages', //overridden
                 'pageSource', // not needed
                 'pageSource', // not needed
-                'windowPerformance' // overriden
+                'windowPerformance' // overridden
             ].join(','),
             ].join(','),
             'include-dirs': [
             'include-dirs': [
                 path.join(__dirname, 'custom_modules/core'),
                 path.join(__dirname, 'custom_modules/core'),

+ 5 - 0
test/www/jquery-page.html

@@ -190,6 +190,11 @@
         $li.parentsUntil('body', 'div');
         $li.parentsUntil('body', 'div');
         $li.siblings();
         $li.siblings();
         $li.siblings('li');
         $li.siblings('li');
+
+        $.ajax({
+            url: 'xml.xml',
+            async: false
+        });
     </script>
     </script>
 </body>
 </body>
 </html>
 </html>