Issue #357: Fixed shareYUIInstance

YahooArchive · Aug 14, 2012 · ea73085 · ea73085
1 parent 00e8d45
commit ea73085
Show file tree

Hide file tree

Showing 6 changed files with 96 additions and 11 deletions.
diff --git a/source/lib/app/autoload/controller-context.common.js b/source/lib/app/autoload/controller-context.common.js
@@ -16,8 +16,7 @@ YUI.add('mojito-controller-context', function(Y, NAME) {
         this.dispatch = opts.dispatch;
         this.store = opts.store;
         this.Y = opts.Y;
-        this.shareYUIInstance = opts.appShareYUIInstance &&
-            this.instance.shareYUIInstance;
+        this.shareYUIInstance = Y.mojito.util.shouldShareYUIInstance(opts.appShareYUIInstance, this.instance);
         this.init();
     }
 
@@ -42,12 +41,19 @@ YUI.add('mojito-controller-context', function(Y, NAME) {
                 c = this.Y.mojito.controller ||
                     this.Y.mojito.controllers[instance['controller-module']];
 
+            // If sharing YUI and controller clobbers, log an error.
+            if (shareYUIInstance && this.Y.mojito.controller) {
+                this.Y.log(instance['controller-module'] + ' mojit' +
+                    ' clobbers Y.mojito.controller namespace. Please use' +
+                    ' `Y.namespace(\'mojito.controllers\')[NAME]` when ' +
+                    ' declaring controllers.', 'error', NAME);
+            }
+
             if (!Y.Lang.isObject(c)) {
                 error = new Error('Mojit controller prototype is not an' +
                     ' object! (mojit id: \'' + instance.id + '\')');
 
-                // TODO: change this to a more appropriate error code.
-                error.code = 404;
+                error.code = 500;
                 throw error;
             }
 

diff --git a/source/lib/app/autoload/dispatch.common.js b/source/lib/app/autoload/dispatch.common.js
@@ -127,7 +127,7 @@ YUI.add('mojito-dispatcher', function(Y, NAME) {
                 // We replace the given instance with the expanded instance
                 command.instance = instance;
 
-                if (appShareYUIInstance && instance.shareYUIInstance) {
+                if (Y.mojito.util.shouldShareYUIInstance(appShareYUIInstance, command.instance)) {
                     instanceYuiCacheKey = 'singleton';
                 } else {
                     // Generate a cache key
@@ -271,7 +271,7 @@ YUI.add('mojito-dispatcher', function(Y, NAME) {
                 }
 
                 // Get the cached YUI instance (if there is one)
-                if (!(appShareYUIInstance && instance.shareYUIInstance)) {
+                if (!Y.mojito.util.shouldShareYUIInstance(appShareYUIInstance, command.instance)) {
                     instanceYuiCacheObj = CACHE.YUI[instanceYuiCacheKey];
                 }
 
@@ -329,7 +329,7 @@ YUI.add('mojito-dispatcher', function(Y, NAME) {
 
             appConfigStatic = store.getAppConfig({});
 
-            appShareYUIInstance = (false !== appConfigStatic.shareYUIInstance);
+            appShareYUIInstance = (true === appConfigStatic.shareYUIInstance);
             usePrecomputed = appConfigStatic.yui && (-1 !==
                 appConfigStatic.yui.dependencyCalculations.indexOf('precomputed'));
             useOnDemand = appConfigStatic.yui && (-1 !==

diff --git a/source/lib/app/autoload/util.common.js b/source/lib/app/autoload/util.common.js
@@ -383,6 +383,33 @@ YUI.add('mojito-util', function(Y) {
                 }
             }
             return url;
+        },
+
+        /**
+         * Determines whether a mojit instance should share its YUI instance.
+         * The app config determines this, however invdividual mojits can
+         * override the app value by setting either true or false. This gives
+         * mojits the ability to opt in, opt out, or leave it up to the application
+         * setting by leaving it undefined.
+         *
+         * @param appShareYUIInstance {boolean} The app's configuration
+         * @param mojitInstance {Object} The mojit instance to check against
+         * @return {boolean}
+         */
+        shouldShareYUIInstance: function (appShareYUIInstance, mojitInstance) {
+            if (true === mojitInstance.shareYUIInstance) {
+                return true;
+            }
+            if (false === mojitInstance.shareYUIInstance) {
+                return false;
+            }
+            if (mojitInstance.defaults && true === mojitInstance.defaults.shareYUIInstance) {
+                return true;
+            }
+            if (mojitInstance.defaults && false === mojitInstance.defaults.shareYUIInstance) {
+                return false;
+            }
+            return appShareYUIInstance || false;
         }
     };
 

diff --git a/source/lib/app/mojits/LazyLoad/controller.common.js b/source/lib/app/mojits/LazyLoad/controller.common.js
@@ -9,9 +9,9 @@
 /*global YUI*/
 
 
-YUI.add('LazyLoad', function(Y) {
+YUI.add('LazyLoad', function(Y, NAME) {
 
-    Y.namespace('mojito').controller = {
+    Y.namespace('mojito.controllers')[NAME] = {
 
         /*
          * Initially, renders a bar node

diff --git a/tests/unit/lib/app/autoload/test-utils.js → ...nit/lib/app/autoload/test-utils.common.js b/tests/unit/lib/app/autoload/test-utils.js → ...nit/lib/app/autoload/test-utils.common.js
@@ -418,6 +418,58 @@
             };
             var result = Y.mojito.util.metaMerge(to, from);
             OA.areEqual(expected['content-type'], result['content-type']);
+        },
+
+        'test shouldShareYUIInstance': function() {
+            var tests = [
+                    {
+                        desc: 'Test Defaults',
+                        appShare: undefined,
+                        instance: {shareYUIInstance: undefined},
+                        expected: false
+                    },
+                    {
+                        desc: 'Test App True',
+                        appShare: true,
+                        instance: {shareYUIInstance: undefined},
+                        expected: true
+                    },
+                    {
+                        desc: 'Test App False',
+                        appShare: false,
+                        instance: {shareYUIInstance: undefined},
+                        expected: false
+                    },
+                    {
+                        desc: 'Instance Override True',
+                        appShare: false,
+                        instance: {shareYUIInstance: true},
+                        expected: true
+                    },
+                    {
+                        desc: 'Instance Override False',
+                        appShare: true,
+                        instance: {shareYUIInstance: false},
+                        expected: false
+                    },
+                    {
+                        desc: 'Instance Override False',
+                        appShare: undefined,
+                        instance: {shareYUIInstance: false},
+                        expected: false
+                    },
+                    {
+                        desc: 'Instance Override True',
+                        appShare: undefined,
+                        instance: {shareYUIInstance: true},
+                        expected: true
+                    }
+                ],
+                result;
+            Y.Array.each(tests, function (test) {
+                result = Y.mojito.util.shouldShareYUIInstance(test.appShare, test.instance);
+                A.areEqual(test.expected, result, test.desc + ' failed.');
+            });
         }
 
     };

diff --git a/tests/unit/lib/app/autoload/test_descriptor.json b/tests/unit/lib/app/autoload/test_descriptor.json
@@ -43,9 +43,9 @@
             "utils": {
                 "params": {
                     "page": "$$config.base$$/mojito-test.html",
-                    "test": "./test-utils.js"
+                    "test": "./test-utils.common.js"
                 },
-                "group": "fw,unit,client"
+                "group": "fw,unit,client,server"
             }
         }
     },