Skip to content
This repository has been archived by the owner on Oct 30, 2018. It is now read-only.

Allow mojits to extend YUI modules in other mojits #1385

Merged
merged 20 commits into from Aug 8, 2014
Merged

Conversation

aljimenez
Copy link
Contributor

This pull request allows mojits to easily extend YUI modules of other mojits. For example:

mojits/Vehicle/controller.server.js

YUI.add('VehicleController', function (Y, NAME) {
    var Vehicle = function () {
        this.type = 'Vehicle';
        this.transmission = 'automatic';
    };

    Y.namespace('mojito.controllers')[NAME] = Vehicle;
    Y.extend(Vehicle, Y.Base, {
        index: function () {
            ac.done(ac.params.body('brand') + ' ' + this.getType());
        },
        getType: function () {
            return this.type;
        }
    });
}, '0.0.1', {
    requires: {
        'oop',
        'mojito-params-addon'
    }
});

mojits/Motorcycle/controller.server.js

YUI.add('MotorcycleController', function (Y, NAME) {
    var Vehicle = Y.mojito.controllers.VehicleController,
        Motorcycle = function () {
            Motorcycle.superclass.constructor.call(this);
            this.type = 'Motorcycle';
            this.transmission = 'manual';
        };

    Y.namespace('mojito.controllers')[NAME] = Motorcycle;
    Y.mojito.util.extend(Motorcycle, Vehicle);
}, '0.0.1', {
    requires: {
        'mojito-util',
        'VehicleController'
    }
});

In this example, the Motorcycle mojit extends the Vehicle mojit, which extends the Y.Base class (extending Y.Base is not a requirement, the example just illustrates multiple level of extensions). Notice that the controllers can be defined as a function; this allows mojits to extend using Y.extend, which ensures that the prototype chain is maintained regardless of how many levels of extensions. Also notice that the Motorcycle controller does not have to re-specify the 'mojito-params-addon' requirement; it just has to specify the Vehicle controller requirement, and its addon will automatically include any addon required by Vehicle controller.

To ensure that mojit dependencies work with resourceStore > lazyMojits on, mojits depending on other mojits should specify their dependencies in defaults.json:

mojits/Motorcycle/defaults.json

[{
    "settings": [ "master" ],
    "dependencies": [ "Vehicle" ]
}]

…extend other controllers through Y.extend, while maintaining the prototype chain.
…The RS addon keeps track of all the controllers and ac addons and then during the resolving of mojit details, it determines all the addons required by a controller by iterating through its ac addon and controller requirements.
…The RS addon keeps track of all the controllers and ac addons and then during the resolving of mojit details, it determines all the addons required by a controller by iterating through its ac addon and controller requirements.
@yahoocla
Copy link

CLA is valid!

@gomezd
Copy link
Collaborator

gomezd commented Jun 27, 2014

IMHO we need to do extensive performance testing to determine any possible impact of this.
And i'm not a big fan of having to declare the dependencies in another file (defaults.json), seems to me confusing and error prone.

@aljimenez
Copy link
Contributor Author

This pull request does not change the execution flow for any existing application. It only adds a new feature, which is to allow a mojit controller to be specified as a function with a prototype. This has no performance impact, since the execution flow remains the same. The only thing that changes is that instead of mojito using Y.mojito.util.heir to create the controller object (see heir), the controller object is created directly from the controller function. This means that we don't have to create a new function every time, which obviously wouldn't degrade performance or memory usage. Specifying the dependency somehow is necessary only if using lazyMojits, there is no way around this, and shouldn't be a huge issue for users since extending the modules of another mojit should be rare. In fact without the pull request mojits extending another mojit's modules would have to specify the dependency through YUI().applyConfig (see ShakerPipelineFrame), in case lazyMojits is on. Specifying the dependencies in default.json is probably the cleanest most intuitive way (and probably good practice anyways since in general mojits are self contained, so its a good idea to mention that it depends on another mojit in its configuration).

@caridy
Copy link
Contributor

caridy commented Jun 30, 2014

This PR is misleading, it is not extending mojits, it is extending individual modules within a mojit, and that's supported today as far as I can tell, by relying on the YUI module definition, I wonder why are you trying to mix the concepts of a mojit and modules in this PR.

@aljimenez
Copy link
Contributor Author

Thats right, you can extend any YUI module of another mojit, including controllers, binders, models... This pull request enhances 3 things:

  1. When you require a controller, you also get the addons of that controller. Before if you required an external controller you would also need to copy the list of addons required by that controller. (See this example)
  2. The pull request allows you to use the Y.extend directly on the controller, since now controllers can be defined as functions. This is important in order to maintain the prototype chain regardless of how deep the level of extending. For example mojit c's controller can extend mojit's b, which extends mojit's a's controller. Previously you would have to use Y.merge since Y.extend only works on functions not object literals. And if you used Y.merge you would lose the prototype chain and mojit c's controller would not properly inherit mojit's a controller methods.
  3. When using lazyMojits, mojits are loaded as they are needed, however if a mojit depends on the YUI module of another mojit, then those YUI modules would not be available. This is currently solved by having a Y.applyConfig statement before doing YUI.add (see ShakerPipelineFrame). Defining the mojit's external dependencies in defaults.json, is a cleaner solution since it doesn't require the applyConfig statement and is also an easy way to see if this mojit depends on another mojit.

@aljimenez aljimenez changed the title Allow mojits to be extended Allow mojits to extend YUI modules in other mojits Jun 30, 2014
@aljimenez
Copy link
Contributor Author

In the latest commit, Y.mojito.util.heir accepts functions in addition to object literals. This allows controllers/binders/models to be defined as functions, which can be useful when extending other YUI modules. The commit also adds Y.mojito.util.extend, a wrapper around Y.extend, in order to accept either functions or object literals as arguments.

…structs it. Added util.extend a wrapper around Y.extend to allow object literals in addition to functions.
…getMojitDetails which recursively loads dependencies. Also making sure that if the mojit is required on the client side that details.modules contains not just its modules but also all the modules of its dependencies.
…ing inheritance by introducing the ColorChild mojit, whose controller/model/binder the Red/Blue/GreenChild mojits extend. Also introduced the PurpleChild mojit which extends modules in both the Red/Blue/ColorChild mojits; this demonstrates multiple levels of inheritance.
// We only care about controllers and AC addons since a mojit's controller's addons
// should be derived from its addons requirements, recursively including the addons of
// the controllers and addons it requires.
this.modules[res.yui.name] = res;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we keep track of all the ac addons and controllers, which we use below in order to populate a details.acAddons which includes all ac addons specified in its requirement tree.

@caridy
Copy link
Contributor

caridy commented Jul 2, 2014

need some time to go over this tomorrow.

@aljimenez
Copy link
Contributor Author

Thanks Caridy, let me know if I should clarify anything.

…loader to the meta, to ensure that after the ajax request, the client has up to date loader meta and knows how to load any newly added binders/langs from lazy loaded mojits/langs.
// Use the loader-app module, to ensure the new loader meta gets applied.
meta.assets.top.blob.push('<script>YUI().use("loader-app");</script>');
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When there is a tunnel request and mojits/langs are loaded lazily, then the loader meta data on the client becomes out of date and so here we update the loader on the server and add the loader to meta.assets such that the client loads the new loader meta.

…th the same name but client/server affinities, duplicate yui names at different levels or with different selectors.
… be the case when using lazyMojits/lazyLangs and executing a mojit without going through the mojito-handler-static middleware (for example through some testing framework like mojito-markup-test)
@aljimenez
Copy link
Contributor Author

@caridy can you review this when you get a chance; we have some application code that depend on this pull request. Thanks.

@caridy
Copy link
Contributor

caridy commented Aug 5, 2014

@aljimenez let's have a chat tomorrow about this PR, I have few specific questions and some suggestions, but this is a very complex PR, hard to make sense of all pieces.

@aljimenez
Copy link
Contributor Author

@caridy sounds good. The pull request did get a bit complicated due to some refactoring and some other fixes that I slipped in, somewhat unrelated to the original pull request. I'll make sure to separate the distinct features in different branches in the future, thanks.

@caridy
Copy link
Contributor

caridy commented Aug 8, 2014

ok, I will not hold back on this: +0.9

@aljimenez
Copy link
Contributor Author

Thanks @caridy I will merge this soon, and I'll keep in mind your suggestions.

aljimenez added a commit that referenced this pull request Aug 8, 2014
Allow mojits to extend YUI modules in other mojits
@aljimenez aljimenez merged commit a710c7e into develop Aug 8, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants