0.5.2 fix, more flexible component instantiation; TODO reuse dom stuff

gadicc · Jun 6, 2015 · 1038da2 · 1038da2
1 parent 632f1b6
commit 1038da2
Show file tree

Hide file tree

Showing 11 changed files with 96 additions and 46 deletions.
diff --git a/HISTORY.md b/HISTORY.md
@@ -1,5 +1,9 @@
 ## vNEXT
 
+* Fix for Famous 0.5.2 (and further work on DOMElement handling)
+* Initial child cleanup handling
+* Support components that don't add themselves to the node + set `_id`
+
 * Fix wrapper with no args' renderFunc using `with` data as args
 * famousEach, in a totally new and extensible way
 * Some internal changes in how nodes are dismounted

diff --git a/lib/meteorFamousView.js b/lib/meteorFamousView.js
@@ -33,11 +33,23 @@ MeteorFamousView = FView._MeteorFamousView =
  * @method
  */
 MeteorFamousView.prototype.destroy = function() {
+  var fview = this;
   delete fviews[this.id];
 
   log.debug("Destroying " + this.type + " (#" + this.id + ") from " + this._source);
 
-  // TODO children, etc.
+  // TODO, tests
+  _.each(this.children, function(child) {
+    //child.destroy();
+    Blaze.remove(child.blazeView);
+  });
+
+  // components
+  // TODO, tests
+  // TODO, is this the right place for this?
+  _.each(this.components, function(compData) {
+    compData.fvClass.destroy.call(compData, fview);
+  });
 
   // remove from parent
   if (this.parent)

diff --git a/lib/utilities.js b/lib/utilities.js
@@ -162,3 +162,10 @@ getElementFromDOMElement = function(node, callback) {
   // except for this :(
   clock.setTimeout(query, 64);
 };
+
+block = function(s) {
+  var ms = s * 1000;
+  var start = performance.now();
+  console.log('blocking for ' + s + 's');
+  while (performance.now() - start < ms);
+}
diff --git a/lib/wrappers/Components.js b/lib/wrappers/Components.js
@@ -3,8 +3,11 @@ var ComponentClass = {
   famousClass: null,
 
   newComponentInstance: function(fview) {
-    var fvClass = this;
-    return new fvClass.famousClass(fview.node);
+    //var fvClass = this;
+    var comp = new this.famousClass(fview.node);
+    if (!(comp._id || comp.id))
+      comp._id = fview.node.addComponent(comp);
+    return comp;
   },
 
   addChild: function(child) {
@@ -73,12 +76,25 @@ var ComponentClass = {
       instance: fvClass.newComponentInstance.call(fvClass, fview)
     };
 
-    fview[fvClass.shortcutName] = fview.components[fvClass.name].instance;
+    if (fvClass.shortcutName)
+      fview[fvClass.shortcutName] = fview.components[fvClass.name].instance;
+  },
+
+  destroy: function(fview) {
+    // dont do this?  some components rely on node unmount
+    //fview.node.removeComponent(this.instance);
   },
 
   templateDestroyed: function() {
-    throw new Error("You tried to remove a Component via Blaze, but you "
-      + "but should remove it's parent node instead");
+    // TODO, untested
+    var blazeView = this.view;
+    var fview = fviewParentFromBlazeView(blazeView);
+    var fvClass = blazeView.template._fviewClass;
+    var componentData = fview.components[fvClass.name];
+    fvClass.destroy.call(componentData, fview);
+
+//    throw new Error("You tried to remove a Component via Blaze, but you "
+//      + "but should remove it's parent node instead");
   },
 
   makeTemplate: function(fvClass) {

diff --git a/lib/wrappers/Components/DOMElement.js b/lib/wrappers/Components/DOMElement.js
@@ -4,7 +4,7 @@ FView.ready(function() {
   var renderedPaths = {};
 
   FView.wrapComponent('DOMElement', famous.domRenderables.DOMElement, {
-    attrUpdate: function(key, value, oldValue, data, firstTime) {
+    attrUpdate: function(key, value, oldValue, data, firstTime, compData) {
       var fview = this;
 
       if (!fview._domElementData) {
@@ -72,13 +72,27 @@ FView.ready(function() {
       }
     },
 
+    destroy: function(fview) {
+      var domRenderer = fview._loadedDomRenderer();
+      delete domRenderer._target.content._fview;
+      Blaze.remove(this.renderedView);
+      this.fvClass._super.destroy.apply(this, arguments);
+    },
+
     templateCreated: function() {
       var blazeView = this.view;
       var fview = fviewParentFromBlazeView(blazeView);
       var fvClass = blazeView.template._fviewClass;
 
-      //fview.domElement = new DOMElement(fview.node);
-      fview.domElement = fvClass.newComponentInstance.call(fvClass, fview);
+      fview.components.DOMElement = {
+        fvClass: fvClass,
+        blazeView: this,
+        instance: fvClass.newComponentInstance.call(fvClass, fview),
+      };
+
+      // shortcut
+      fview.domElement = fview.components.DOMElement.instance;
+
       var path = fview.node.getId();
       var context = path.split('/', 1)[0];
       var domRenderer = FamousEngine.compositor.getOrSetContext(context).DOMRenderer;
@@ -99,7 +113,7 @@ FView.ready(function() {
       };
 
       fview.updateSizeDeferred = function() {
-        _.defer(function() {
+        FView.defer(function() {
           fview.updateSize();
         });
       };
@@ -122,15 +136,20 @@ FView.ready(function() {
             // Remove when merged: https://github.com/Famous/engine/pull/58
             domRenderer.findChildren();
 
-            domRenderer._target.content = document.createElement('div');
-            domRenderer._target.content.classList.add('famous-dom-element-content');
-            domRenderer._target.element.insertBefore(
-                domRenderer._target.content,
-                domRenderer._target.element.firstChild
-            );
+            if (!domRenderer._target.content) {
+              domRenderer._target.content = document.createElement('div');
+              domRenderer._target.content.classList.add('famous-dom-element-content');
+              domRenderer._target.element.insertBefore(
+                  domRenderer._target.content,
+                  domRenderer._target.element.firstChild
+              );
+            }
+            // make sure we don't rerender to an existing element
             domRenderer._target.content._fview = true;
 
-            Blaze.render(viewToRender, domRenderer._target.content, blazeView);
+            // Store renderedView ref for cleanup on distroy()
+            fview.components[fvClass.name].renderedView =
+              Blaze.render(viewToRender, domRenderer._target.content, blazeView);
 
             domRenderer.setSize(
               domRenderer._target.explicitWidth ? false : domRenderer._target.size[0],

diff --git a/lib/wrappers/Nodes/Scene.js b/lib/wrappers/Nodes/Scene.js
@@ -19,7 +19,9 @@ FView.wrap('Scene', null, {
   },
 
   dismount: function() {
-    this.node.dismount();
+    // FamousEngine does not destroy scenes #182
+    // https://github.com/Famous/engine/issues/182
+    this._scene.dismount();
   },
 
   renderFunc: function() {
@@ -91,15 +93,5 @@ FView.wrap('Scene', null, {
         log.error("No such helper for _onRender: " + data._onRender);
       delete data._onRender;
     }
-  },
-
-  templateDestroy: function() {
-    var fview = FView.from(this);
-    fview.destroy();
-
-    // FamousEngine does not destroy scenes #182
-    // https://github.com/Famous/engine/issues/182
-    fview._scene.dismount();    
-  },
-
+  }
 });
diff --git a/package.js b/package.js
@@ -21,7 +21,7 @@ function common(api) {
   ], client)
 
   // Famous
-  api.use('gadicohen:[email protected].0_4', client);
+  api.use('gadicohen:[email protected].2', client);
 
   api.addFiles([
     'lib/famous-views.js',

diff --git a/tests/famousEach.js b/tests/famousEach.js
@@ -20,7 +20,7 @@ Template.famousEachTest.helpers({
 
 Tinytest.add('famous-views - famousEach - setup', function(test) {
   // maintain order
-  Blaze.render(Template.famousEachTest, commonDiv);
+  Blaze.render(Template.famousEachTest, testDiv());
 });
 
 Tinytest.addAsync('famous-views - famousEach - addedAt append', function(test, complete) {

diff --git a/tests/lib/prepare.js b/tests/lib/prepare.js
@@ -1,8 +1,8 @@
-commonDiv = null;
-Meteor.startup(function() {
-  commonDiv = document.createElement('div');
-  commonDiv.style.display = 'none';
-  document.body.appendChild(commonDiv);
-});
+noop = function() {};
 
-noop = function() {};
+testDiv = function() {
+  var div = document.createElement('div');
+  div.style.display = 'none';
+  document.body.appendChild(div);  
+  return div;
+}
diff --git a/tests/wrappers/DOMElement.js b/tests/wrappers/DOMElement.js
@@ -6,7 +6,7 @@ Tinytest.addAsync('famous-views - Wrappers - DOMElement - content', function(tes
       complete();
     });
   }
-  Blaze.render(Template.domEl1, commonDiv);
+  Blaze.render(Template.domEl1, testDiv());
 });
 
 Tinytest.addAsync('famous-views - Wrappers - DOMElement - content on repurposed el', function(test, complete) {
@@ -30,7 +30,7 @@ Tinytest.addAsync('famous-views - Wrappers - DOMElement - content on repurposed
   Template.domEl2.helpers({
     x: function() { return x.get(); }
   });
-  Blaze.render(Template.domEl2, commonDiv);
+  Blaze.render(Template.domEl2, testDiv());
 });
 
 
@@ -41,7 +41,7 @@ Tinytest.add('famous-views - Wrappers - DOMElement - attributes - classes', func
     getClasses: function() { return classes.get(); }
   });
 
-  Blaze.render(Template.domEl_classes, commonDiv);
+  Blaze.render(Template.domEl_classes, testDiv());
   Tracker.flush();
 
   var fview = FView.byId("domEl_classes");
@@ -64,7 +64,7 @@ Tinytest.add('famous-views - Wrappers - DOMElement - attributes - style', functi
     reactiveStyle: function() { return style.get(); }
   });
 
-  Blaze.render(Template.domEl_style, commonDiv);
+  Blaze.render(Template.domEl_style, testDiv());
   Tracker.flush();
 
   var DE = FView.byId("domEl_style").domElement;
@@ -88,7 +88,7 @@ Tinytest.add('famous-views - Wrappers - DOMElement - attributes - other', functi
     getLang: function() { return lang.get(); }
   });
 
-  Blaze.render(Template.domEl_attributes, commonDiv);
+  Blaze.render(Template.domEl_attributes, testDiv());
   Tracker.flush();
 
   var DE = FView.byId("domEl_attributes").domElement;

diff --git a/tests/wrappers/Node.js b/tests/wrappers/Node.js
@@ -66,7 +66,7 @@ Tinytest.addAsync('famous-views - Wrappers - Node - _onRender', function(test, c
     }
   });
 
-  Blaze.render(Template.node2, commonDiv);
+  Blaze.render(Template.node2, testDiv());
 });
 
 // also tests: addToParent, addChild
@@ -79,7 +79,7 @@ Tinytest.addAsync('famous-views - Wrappers - Node - template create', function(t
     test.equal(scene.node._children.indexOf(node.node), 0);
     complete();
   };
-  Blaze.render(Template.node3, commonDiv);
+  Blaze.render(Template.node3, testDiv());
 });
 
 // also tests: dismount
@@ -100,5 +100,5 @@ Tinytest.addAsync('famous-views - Wrappers - Node - template destroy', function(
     });
     x.set(false);
   };
-  Blaze.render(Template.node4, commonDiv);
+  Blaze.render(Template.node4, testDiv());
 });