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);
> + }
> +}
>
>
>
>
>
>
>
>
>
>
>
>
