Skip to content

Commit

Permalink
Added support for destroy(true) to Widget, which will remove and
Browse files Browse the repository at this point in the history
destroy all child nodes (not just the boundingBox and contentBox)
contained within the Widget's boundingBox in order to help control
Node cache size over long-running applications.

destroy() will maintain it's current behavior due to the potentially
high run-time cost of destroying all child nodes.

Widget developers still need to continue with the best practice
of destroying explicit node references they create, in their
destructors to support the destroy() case.

If you destroy Widgets in your application or are a custom widget
developer, your help in testing this change before release is much
appreciated.

Fixes #2530444
  • Loading branch information
sdesai committed Jul 23, 2011
1 parent 2e7fa43 commit d8f2b0d
Show file tree
Hide file tree
Showing 9 changed files with 157 additions and 23 deletions.
2 changes: 0 additions & 2 deletions build/base-base/base-base-debug.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ YUI.add('base-base', function(Y) {
* invoking initializers for the class hierarchy.
*
* @method init
* @final
* @chainable
* @param {Object} config Object with configuration property name/value pairs
* @return {Base} A reference to this object
Expand Down Expand Up @@ -279,7 +278,6 @@ YUI.add('base-base', function(Y) {
* </p>
* @method destroy
* @return {Base} A reference to this object
* @final
* @chainable
*/
destroy: function() {
Expand Down
2 changes: 0 additions & 2 deletions build/base-base/base-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ YUI.add('base-base', function(Y) {
* invoking initializers for the class hierarchy.
*
* @method init
* @final
* @chainable
* @param {Object} config Object with configuration property name/value pairs
* @return {Base} A reference to this object
Expand Down Expand Up @@ -277,7 +276,6 @@ YUI.add('base-base', function(Y) {
* </p>
* @method destroy
* @return {Base} A reference to this object
* @final
* @chainable
*/
destroy: function() {
Expand Down
39 changes: 34 additions & 5 deletions build/widget-base/widget-base-debug.js
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,29 @@ Y.extend(Widget, Y.Base, {
this._destroyBox();
},

/**
* <p>
* Destroy lifecycle method. Fires the destroy
* event, prior to invoking destructors for the
* class hierarchy.
*
* Overrides Base's implementation, to support arguments to destroy
* </p>
* <p>
* Subscribers to the destroy
* event can invoke preventDefault on the event object, to prevent destruction
* from proceeding.
* </p>
* @method destroy
* @param destroyAllNodes {Boolean} If true, all nodes contained within the Widget are removd and destroyed. Defaults to false due to potentially high run-time cost.
* @return {Widget} A reference to this object
* @chainable
*/
destroy: function(destroyAllNodes) {
this._destroyAllNodes = destroyAllNodes;
return Widget.superclass.destroy.apply(this);
},

/**
* Removes and destroys the widgets rendered boundingBox, contentBox,
* and detaches bound UI events.
Expand All @@ -437,6 +460,7 @@ Y.extend(Widget, Y.Base, {

var boundingBox = this.get(BOUNDING_BOX),
contentBox = this.get(CONTENT_BOX),
deep = this._destroyAllNodes,
same = boundingBox && boundingBox.compareTo(contentBox);

if (this.UI_EVENTS) {
Expand All @@ -445,12 +469,17 @@ Y.extend(Widget, Y.Base, {

this._unbindUI(boundingBox);

if (contentBox) {
contentBox.remove(TRUE);
}

if (!same) {
if (deep) {
// Removes and destroys all child nodes.
boundingBox.empty();
boundingBox.remove(TRUE);
} else {
if (contentBox) {
contentBox.remove(TRUE);
}
if (!same) {
boundingBox.remove(TRUE);
}
}
},

Expand Down
2 changes: 1 addition & 1 deletion build/widget-base/widget-base-min.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

39 changes: 34 additions & 5 deletions build/widget-base/widget-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,29 @@ Y.extend(Widget, Y.Base, {
this._destroyBox();
},

/**
* <p>
* Destroy lifecycle method. Fires the destroy
* event, prior to invoking destructors for the
* class hierarchy.
*
* Overrides Base's implementation, to support arguments to destroy
* </p>
* <p>
* Subscribers to the destroy
* event can invoke preventDefault on the event object, to prevent destruction
* from proceeding.
* </p>
* @method destroy
* @param destroyAllNodes {Boolean} If true, all nodes contained within the Widget are removd and destroyed. Defaults to false due to potentially high run-time cost.
* @return {Widget} A reference to this object
* @chainable
*/
destroy: function(destroyAllNodes) {
this._destroyAllNodes = destroyAllNodes;
return Widget.superclass.destroy.apply(this);
},

/**
* Removes and destroys the widgets rendered boundingBox, contentBox,
* and detaches bound UI events.
Expand All @@ -434,6 +457,7 @@ Y.extend(Widget, Y.Base, {

var boundingBox = this.get(BOUNDING_BOX),
contentBox = this.get(CONTENT_BOX),
deep = this._destroyAllNodes,
same = boundingBox && boundingBox.compareTo(contentBox);

if (this.UI_EVENTS) {
Expand All @@ -442,12 +466,17 @@ Y.extend(Widget, Y.Base, {

this._unbindUI(boundingBox);

if (contentBox) {
contentBox.remove(TRUE);
}

if (!same) {
if (deep) {
// Removes and destroys all child nodes.
boundingBox.empty();
boundingBox.remove(TRUE);
} else {
if (contentBox) {
contentBox.remove(TRUE);
}
if (!same) {
boundingBox.remove(TRUE);
}
}
},

Expand Down
2 changes: 0 additions & 2 deletions src/base/js/Base.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@
* invoking initializers for the class hierarchy.
*
* @method init
* @final
* @chainable
* @param {Object} config Object with configuration property name/value pairs
* @return {Base} A reference to this object
Expand Down Expand Up @@ -277,7 +276,6 @@
* </p>
* @method destroy
* @return {Base} A reference to this object
* @final
* @chainable
*/
destroy: function() {
Expand Down
5 changes: 5 additions & 0 deletions src/widget/HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ Widget
* Fixed UI_EVENTS invoking nested widget listeners more than once (also
fixed regression to Parent-Child as a result of this change).

* Added destroy(true) support, to remove and destroy all Nodes contained
within a widget's boundingBox. This is useful for long-lived pages,
to limit the growth of Node cache. By default Widget only removes and
destroys the Nodes it references - the boundingBox and contentBox.

3.3.0
-----

Expand Down
39 changes: 34 additions & 5 deletions src/widget/js/Widget.js
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,29 @@ Y.extend(Widget, Y.Base, {
this._destroyBox();
},

/**
* <p>
* Destroy lifecycle method. Fires the destroy
* event, prior to invoking destructors for the
* class hierarchy.
*
* Overrides Base's implementation, to support arguments to destroy
* </p>
* <p>
* Subscribers to the destroy
* event can invoke preventDefault on the event object, to prevent destruction
* from proceeding.
* </p>
* @method destroy
* @param destroyAllNodes {Boolean} If true, all nodes contained within the Widget are removd and destroyed. Defaults to false due to potentially high run-time cost.
* @return {Widget} A reference to this object
* @chainable
*/
destroy: function(destroyAllNodes) {
this._destroyAllNodes = destroyAllNodes;
return Widget.superclass.destroy.apply(this);
},

/**
* Removes and destroys the widgets rendered boundingBox, contentBox,
* and detaches bound UI events.
Expand All @@ -435,6 +458,7 @@ Y.extend(Widget, Y.Base, {

var boundingBox = this.get(BOUNDING_BOX),
contentBox = this.get(CONTENT_BOX),
deep = this._destroyAllNodes,
same = boundingBox && boundingBox.compareTo(contentBox);

if (this.UI_EVENTS) {
Expand All @@ -443,12 +467,17 @@ Y.extend(Widget, Y.Base, {

this._unbindUI(boundingBox);

if (contentBox) {
contentBox.remove(TRUE);
}

if (!same) {
if (deep) {
// Removes and destroys all child nodes.
boundingBox.empty();
boundingBox.remove(TRUE);
} else {
if (contentBox) {
contentBox.remove(TRUE);
}
if (!same) {
boundingBox.remove(TRUE);
}
}
},

Expand Down
Loading

0 comments on commit d8f2b0d

Please sign in to comment.