Show warning for reference types in attributes

Also improves debug warnings for unserializable data and unused attrs. Fix #333.
flightjs · Jan 22, 2015 · 66164d9 · 66164d9
1 parent 039d3c3
commit 66164d9
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 11 deletions.
diff --git a/lib/base.js b/lib/base.js
@@ -30,16 +30,20 @@ define(
       try {
         window.postMessage(data, '*');
       } catch (e) {
-        console.warn([
-          'The event',
-          type,
-          'on component',
-          this.toString(),
-          'was triggered with non-serializable data'
-        ].join(' '));
+        debug.warn.call(this, [
+          'Event "', type, '" was triggered with non-serializable data. ',
+          'Flight recommends you avoid passing non-serializable data in events.'
+        ].join(''));
       }
     }
 
+    function warnAboutReferenceType(key) {
+      debug.warn.call(this, [
+        'Attribute "', key, '" defaults to an array or object. ',
+        'Enclose this in a function to avoid sharing between component instances.'
+      ].join(''));
+    }
+
     function initAttributes(attrs) {
       var definedKeys = [], incomingKeys;
 
@@ -53,8 +57,7 @@ define(
 
         for (var i = incomingKeys.length - 1; i >= 0; i--) {
           if (definedKeys.indexOf(incomingKeys[i]) == -1) {
-            console.warn('Passed unused attributes including "' + incomingKeys[i] +
-                         '" to component "' + this.toString() + '".');
+            debug.warn.call(this, 'Passed unused attribute "' + incomingKeys[i] + '".');
             break;
           }
         }
@@ -66,6 +69,10 @@ define(
             throw new Error('Required attribute "' + key +
                             '" not specified in attachTo for component "' + this.toString() + '".');
           }
+          // Warn about reference types in attributes
+          if (debug.enabled && typeof this.attr[key] === 'object') {
+            warnAboutReferenceType.call(this, key);
+          }
         } else {
           this.attr[key] = attrs[key];
         }
@@ -85,6 +92,10 @@ define(
       for (var key in this.defaults) {
         if (!attrs.hasOwnProperty(key)) {
           attr[key] = this.defaults[key];
+          // Warn about reference types in defaultAttrs
+          if (debug.enabled && typeof this.defaults[key] === 'object') {
+            warnAboutReferenceType.call(this, key);
+          }
         }
       }
 

diff --git a/lib/component.js b/lib/component.js
@@ -16,6 +16,10 @@ define(
     'use strict';
 
     var functionNameRegEx = /function (.*?)\s?\(/;
+    var ignoredMixin = {
+      withBase: true,
+      withLogging: true
+    };
 
     // teardown for all instances of this constructor
     function teardownAll() {
@@ -64,9 +68,8 @@ define(
           // function name property not supported by this browser, use regex
           var m = mixin.toString().match(functionNameRegEx);
           return (m && m[1]) ? m[1] : '';
-        } else {
-          return (mixin.name != 'withBase') ? mixin.name : '';
         }
+        return (!ignoredMixin[mixin.name] ? mixin.name : '');
       }).filter(Boolean).join(', ');
     };
 

diff --git a/lib/debug.js b/lib/debug.js
@@ -133,6 +133,12 @@ define(
         window.DEBUG = this;
       },
 
+      warn: function (message) {
+        if (!window.console) { return; }
+        var fn = (console.warn || console.log);
+        fn.call(console, this.toString() + ': ' + message);
+      },
+
       registry: registry,
 
       find: {