marionettejs / marionette.inspector

:mag: Marionette Inspector - Explore your App
http://goo.gl/Wo3pju
MIT License
170 stars 42 forks source link

Show View life-cycle warnings #107

Closed jasonLaster closed 9 years ago

jasonLaster commented 9 years ago

Problem

One of the things that the Inspector could do really well is show you when bad things happen during the course of using your application. Two very common problems are views being too aggressively rendered (rendered many times in a second) and renders taking too long (a single render taking >500ms).

The inspector logs all of the view and model trigger events and can report when these bad things happen. We could collect these warnings in the UI View info panel in a separate warnings pane.


Implementation


Patch

patch

curl https://gist.githubusercontent.com/jasonLaster/b2de66163eca11d6307e/raw/9ade101522379797e657547d0f3846aa30e02b59/foo.diff |  g apply
diff --git a/extension/js/agent/patches/patchAppComponentTrigger.js b/extension/js/agent/patches/patchAppComponentTrigger.js
index 6e456f4..bd30bda 100644
--- a/extension/js/agent/patches/patchAppComponentTrigger.js
+++ b/extension/js/agent/patches/patchAppComponentTrigger.js
@@ -9,8 +9,10 @@ var patchAppComponentTrigger = bind(function(appComponent, eventType) {//
     patchFunctionLater(appComponent, "trigger", function(originalFunction) {
       return function(eventName, component) {

+        var startTime = Data.new();
         agent.depth++;
         var result = originalFunction.apply(this, arguments);
+        var stopTime = Date.new();

         // function signature: trigger(eventName, arg1, arg2, ...)
         //
@@ -24,7 +26,10 @@ var patchAppComponentTrigger = bind(function(appComponent, eventType) {//
             args: _.map(args, agent.inspectValue, agent),
             depth: agent.depth,
             cid: this.cid,
-            context: agent.inspectValue(context)
+            context: agent.inspectValue(context),
+            startTime: startTime.toString(),
+            stopTime: stopTime.toString(),
+            duration: stopTime - startTime,
         };
         var dataKind = (data === undefined) ? undefined : "event arguments";

diff --git a/extension/js/inspector/app/modules/Activity/models/ActivityCollection.js b/extension/js/inspector/app/modules/Activity/models/ActivityCollection.js
index 53e46b7..413c5a7 100644
--- a/extension/js/inspector/app/modules/Activity/models/ActivityCollection.js
+++ b/extension/js/inspector/app/modules/Activity/models/ActivityCollection.js
@@ -4,7 +4,13 @@ define([
 ], function(Backbone, ActivityModel) {

   return Backbone.Collection.extend({
-    model: ActivityModel
+    model: ActivityModel,
+
+    byTargetCid: function(cid){
+      return this.filter(function(activityModel) {
+        return activityModel.get('data').cid == cid;
+      });
+    }
   });

 })
\ No newline at end of file
diff --git a/extension/js/inspector/app/modules/UI/views/ViewMoreInfo.js b/extension/js/inspector/app/modules/UI/views/ViewMoreInfo.js
index ebe3a48..e62843e 100644
--- a/extension/js/inspector/app/modules/UI/views/ViewMoreInfo.js
+++ b/extension/js/inspector/app/modules/UI/views/ViewMoreInfo.js
@@ -137,6 +137,22 @@ define([
       return ancestors;
     },

+    presentWarnings: function() {
+      var viewCid = this.model.get('cid');
+      var activityForView = Radio.request('Activity', 'targetActivity', viewCid);
+
+      var renderEvents = _.filter(activityForView, function(activity) {
+        return activity.eventName == 'render'
+      });
+
+      // calculate when more than 2 events happened in
+      // less than 50 milliseconds
+      var wasAggressivelyRendered = true;
+
+      // determine if a render took too long
+      var wasSlowlyRendered = true;
+    },
+
     serializeData: function() {
       var infoItems = ['cid', 'model', 'collection', 'parentClass', 'tagName', 'template'];
       var data = {};
@@ -154,6 +170,8 @@ define([
       data.events = this.presentEvents(this.model);
       data.ui = this.presentUI(this.model.get('ui'));
       data.option_key = "options";
+
+      var.warnings = this.presentWarnings();
       return data;
     }
   });
diff --git a/extension/js/inspector/main.js b/extension/js/inspector/main.js
index a07787b..68093f1 100644
--- a/extension/js/inspector/main.js
+++ b/extension/js/inspector/main.js
@@ -86,7 +86,7 @@ require([
         app.module('Data', DataApp);
         app.module('Radio', RadioApp);
         app.module('UI', UIApp);
-        // app.module('Activity', ActivityApp);
+        app.module('Activity', ActivityApp);
     });

     logger.log('devtools', 'started!')
diff --git a/extension/templates/devTools/appLayout.html b/extension/templates/devTools/appLayout.html
index 50cd167..c34b6c3 100644
--- a/extension/templates/devTools/appLayout.html
+++ b/extension/templates/devTools/appLayout.html
@@ -29,11 +29,11 @@
     <li data-tool='radio' class="{{#active_tool.is_radio}}active{{/active_tool.is_radio}}">
       <a data-route="radio" href="#">Radio</a>
     </li>
-    <!--
+
     <li data-tool='activity' class="{{#active_tool.is_activity}}active{{/active_tool.is_activity}}">
       <a data-route="activity" href="#">Activity</a>
     </li>
-    -->
+
   </ul>

 </nav>

People

@joshbedo - currently working on this @jasonLaster - advising on the project

jamesplease commented 9 years ago

:trumpet: