I started just adding to the .prototype directly but then I thought about
x number of features doing this and decided it would be cleaner if they
had the ability to add their specific functionality in their own
namespace. Of course that only my opinion :)
The only place I can think of to add a static function would be in
shindig.container.utils. So maybe something like this, which would hide
the use of the variable.
shindig.container.utils.addContainerMixin = function(namespace, func){
shindig.container.Container.prototype.mixins_[namespace] = func;
}
-Ryan
Email: [email protected]
Phone: 978-899-3041
developerWorks Profile
From: John Hjelmstad <[email protected]>
To: [email protected],
Cc: Ryan J Baxter/Westford/IBM@Lotus
Date: 03/08/2011 02:31 PM
Subject: Re: Allow extensions to the common container by allowing
other features to provide their own namespace (issue4260049)
Makes sense. Why again did you not want extension features to just add to
.prototype directly?
At the very least I'd suggest a "static" method to hide the mixins_ member
variable. I'd prefer a method to a variable into which you stuff content
since it defines the API better.
So something like:
opensocial.container.Container.addMixin = function(mixin) {
// add mixin somewhere, which gets merged on ctor call
};
--j
On Tue, Mar 8, 2011 at 11:04 AM, Ryan J Baxter <[email protected]>
wrote:
> We are in the process of writing several new features that would like to
> extend the common container with some new functionality. But lets say I
> am writing a new feature called foo. In foo's container code in the
> feature I would have to do something like this
>
> shindig.container.Container.prototype.mixins_['foo'] =
function(context){
> return {
> "fooMethod" : function(){
> ......
> }
> };
> }
>
> On my page, if I include the common container feature and my foo feature
I
> would be able to do something like this
>
> var container = new shindig.container.Container();
> container.foo.fooMethod();
>
> The mixins method gets called by the constructor of Container and just
> adds the methods to the Container object, but only if foo feature is
> included on the page.
>
> Hope that makes sense.
>
> -Ryan
>
> Email: [email protected]
> Phone: 978-899-3041
> developerWorks Profile
>
>
>
> From: John Hjelmstad <[email protected]>
> To: [email protected],
> Cc: Ryan J Baxter/Westford/IBM@Lotus
> Date: 03/08/2011 01:53 PM
> Subject: Re: Allow extensions to the common container by allowing
> other features to provide their own namespace (issue4260049)
>
>
>
> Well, what API did you want to expose for extension?
>
> By my (perhaps too quick) read of this code you'd have to create a
> Container object, then do container.mixins_ = { bunch of mixins };, then
> call containerObj.mixin_();
>
> In such case, may as well isolate state and just have the mixin method
> itself take a method.
>
> --j
>
> On Tue, Mar 8, 2011 at 10:44 AM, Ryan J Baxter <[email protected]>
> wrote:
> John, I am not so clear on what your suggesting. I understand you don't
> like the variable, but I don't see how you could execute a method to do
> the "mixin" on the Container object when it has not been created yet. I
> want a feature to be able to add it's own methods to the Container
object
> in its own namespace. So the mixins_ variable would only be used by
other
> features.
>
> -Ryan
>
> Email: [email protected]
> Phone: 978-899-3041
> developerWorks Profile
>
>
>
> From: John Hjelmstad <[email protected]>
> To: [email protected],
> Cc: Ryan J Baxter/Westford/IBM@Lotus
> Date: 03/08/2011 12:59 PM
> Subject: Re: Allow extensions to the common container by allowing
> other features to provide their own namespace (issue4260049)
>
>
>
> Hi Ryan:
>
> The pattern of exposing a magical variable that's read when you call
> this.mixin_() seems a little odd to me. I'd prefer to have something
more
> direct, ie have the mixin method itself take a method/function (which we
> could define as being bound to "this" if you like).
>
>
> --j
>
> On Tue, Mar 8, 2011 at 9:36 AM, Ryan J Baxter <[email protected]>
wrote:
> Anyone have any objections to this? I would like to get this submitted.
>
> -Ryan
>
> Email: [email protected]
> Phone: 978-899-3041
> developerWorks Profile
>
>
>
> From: [email protected]
> To: [email protected], [email protected],
> [email protected],
> Cc: [email protected], [email protected]
> Date: 03/04/2011 11:36 AM
> Subject: Allow extensions to the common container by allowing
other
> features to provide their own namespace (issue4260049)
>
>
>
> Reviewers: mhermanto, dev_shindig.apache.org,
> dev-remailer_shindig.apache.org,
>
> Description:
> At the moment the common container cannot be extended by adding a new
> namespace and still have access to the container object. The only
> reasonable way to extend the common container at this moment it to add
> new methods by calling .prototype or by subclassing it. Another useful
> extension would be to allow features to add their own namespace to the
> common container and add new methods under that common container.
> However those methods may still want to reference other methods inside
> the common container so they need to be able to access the common
> container object itself.
>
> Please review this at http://codereview.appspot.com/4260049/
>
> Affected files:
> features/src/main/javascript/features/container/container.js
> features/src/test/javascript/features/container/container_test.js
>
>
> ### Eclipse Workspace Patch 1.0
> #P shindig-project
> Index: features/src/test/javascript/features/container/container_test.js
> ===================================================================
> --- features/src/test/javascript/features/container/container_test.js
> (revision 1078023)
> +++ features/src/test/javascript/features/container/container_test.js
> (working copy)
> @@ -112,6 +112,22 @@
> this.assertTrue(container.sites_[2] != null);
> };
>
> +ContainerTest.prototype.testMixins_ = function(){
> + this.setupGadgetsRpcRegister();
> + shindig.container.Container.prototype.mixins_['test'] =
> function(context){
> + return {
> + "getSitesLength" :
> function(){
> + return
> context.sites_.length;
> + }
> + };
> + };
> + var container = new shindig.container.Container();
> + this.setupGadgetSite(1, {}, null);
> + container.newGadgetSite(null);
> + this.assertTrue(container.sites_[1] != null);
> + this.assertEquals(container.sites_.length,
> container.test.getSitesLength());
> +};
> +
> ContainerTest.prototype.setupGadgetSite = function(id, gadgetInfo,
> gadgetHolder) {
> var self = this;
> shindig.container.GadgetSite = function() {
> @@ -137,4 +153,4 @@
> register: function() {
> }
> };
> -};
> +};
> \ No newline at end of file
> Index: features/src/main/javascript/features/container/container.js
> ===================================================================
> --- features/src/main/javascript/features/container/container.js
> (revision
> 1078023)
> +++ features/src/main/javascript/features/container/container.js
(working
>
> copy)
> @@ -113,6 +113,8 @@
> * @private
> */
> this.tokenRefreshTimer_ = null;
> +
> + this.mixin_();
>
> this.preloadFromConfig_(config);
>
> @@ -121,7 +123,6 @@
> this.onConstructed(config);
> };
>
> -
> /**
> * Create a new gadget site.
> * @param {Element} gadgetEl HTML element into which to render.
> @@ -636,3 +637,18 @@
> }
> });
> };
> +
> +/**
> + * Adds the ability for features to extend the container with
> + * their own functionality that may be specific to that feature.
> + */
> +shindig.container.Container.prototype.mixins_ = {};
> +
> +/**
> + * Called from the constructor to add any namespace extensions.
> + */
> +shindig.container.Container.prototype.mixin_ = function(){
> + for(var i in this.mixins_){
> + this[i] = new this.mixins_[i](this);
> + }
> +}
>
>
>
>
>
>
>
>
>
>
>
>
