diff --git a/src/loader/HISTORY.md b/src/loader/HISTORY.md index 7ee92c845a8..42e9e897f04 100644 --- a/src/loader/HISTORY.md +++ b/src/loader/HISTORY.md @@ -1,10 +1,15 @@ YUI Loader Change History ========================= -@VERSION@ +3.18.0 ------ -* No changes. +* Inherit group settings from base if they were not specified. ([#1838][]: @andrewnicols) + +3.17.2 +------ + +* Fix a bug in 3.17.1 where there comboBase was no longer inheritted from the default group. ([#1837][]: @andrewnicols) 3.17.1 ------ diff --git a/src/loader/js/loader.js b/src/loader/js/loader.js index 9b21225ebdc..d7346a338b3 100644 --- a/src/loader/js/loader.js +++ b/src/loader/js/loader.js @@ -2637,6 +2637,7 @@ Y.log('Undefined module: ' + mname + ', matched a pattern: ' + var inserted = (self.ignoreRegistered) ? {} : self.inserted, comboSources = {}, maxURLLength, + combine, comboMeta, comboBase, comboSep, @@ -2654,10 +2655,17 @@ Y.log('Undefined module: ' + mname + ', matched a pattern: ' + group = self.groups[mod.group]; + // Set sensible defaults for these on each iteration to ensure we don't leak. + combine = self.combine; comboBase = self.comboBase; + comboSep = self.comboSep; + maxURLLength = self.maxURLLength; if (group) { - if (!group.combine || mod.fullpath) { + if (typeof group.combine === 'boolean') { + combine = group.combine; + } + if (!combine || mod.fullpath) { //This is not a combo module, skip it and load it singly later. addSingle(mod); continue; @@ -2668,9 +2676,18 @@ Y.log('Undefined module: ' + mname + ', matched a pattern: ' + mod.root = group.root; } - comboBase = group.comboBase; - comboSep = group.comboSep; - maxURLLength = group.maxURLLength; + if (typeof group.comboBase === 'string') { + comboBase = group.comboBase; + } + if (comboBase !== self.comboBase) { + // It only makes sense to use a different combo loader settings if the comboBase is different. + if (typeof group.comboSep === 'string') { + comboSep = group.comboSep; + } + if (typeof group.maxURLLength !== 'undefined') { + maxURLLength = group.maxURLLength; + } + } } else { if (!self.combine) { //This is not a combo module, skip it and load it singly later. diff --git a/src/loader/tests/unit/assets/loader-tests.js b/src/loader/tests/unit/assets/loader-tests.js index b90cacf7444..13df39660ee 100644 --- a/src/loader/tests/unit/assets/loader-tests.js +++ b/src/loader/tests/unit/assets/loader-tests.js @@ -312,6 +312,164 @@ YUI.add('loader-tests', function(Y) { Assert.isTrue(out.js.indexOf('http://secondhost.com/combo?3.5.0/foogg/foogg-min.js') >= 0, 'Group combo URL should be included in the result'); Assert.isTrue(out.js.indexOf('http://yui.yahooapis.com/combo?3.5.0/cookie/cookie-min.js') >= 0, 'Default YUI combo URL should be included in the result'); }, + 'test inheritted comboBase with groups': function () { + var loader = new testY.Loader({ + combine: true, + groups: { + testGroup: { + modules: { + foogg: { + requires: [] + } + } + } + }, + require: ['foogg', 'cookie'] + }); + var out = loader.resolve(true); + Assert.areSame(1, out.js.length, 'Loader generated multiple URLs for a single comboBase'); + + var url = out.js[0]; + Assert.isArray(url.match(/3.5.0\/foogg\/foogg-min\.js/), 'Group match should be combo-loaded with the default URL'); + Assert.isArray(url.match(/3.5.0\/cookie\/cookie-min\.js/), 'Default match should combo-load with the group result'); + }, + 'test combine with groups': function () { + var loader = new testY.Loader({ + combine: false, + groups: { + testGroup: { + combine: true, + comboBase: 'http://secondhost.com/combo?', + modules: { + foogg: { + requires: [] + } + } + } + }, + require: ['foogg', 'cookie'] + }); + var out = loader.resolve(true); + Assert.areSame(2, out.js.length, 'Loader did not generate one URL per comboBase'); + Assert.isTrue(out.js.indexOf('http://secondhost.com/combo?3.5.0/foogg/foogg-min.js') >= 0, 'Group combo URL should be included in the result'); + Assert.isTrue(out.js.indexOf('http://yui.yahooapis.com/3.5.0/cookie/cookie-min.js') >= 0, 'Default YUI combo URL should be included in the result'); + }, + 'test inheritted combine with groups': function () { + var loader = new testY.Loader({ + combine: true, + groups: { + testGroup: { + modules: { + foogg: { + requires: [] + } + } + } + }, + require: ['foogg', 'cookie'] + }); + var out = loader.resolve(true); + Assert.areSame(1, out.js.length, 'Loader generated multiple URLs for a single comboBase'); + + var url = out.js[0]; + Assert.isArray(url.match(/3.5.0\/foogg\/foogg-min\.js/), 'Group match should be combo-loaded with the default URL'); + Assert.isArray(url.match(/3.5.0\/cookie\/cookie-min\.js/), 'Default match should combo-load with the group result'); + }, + 'test inheritted comboSep with groups': function () { + var loader = new testY.Loader({ + combine: true, + comboSep: '__PLACEHOLDER__', + groups: { + testGroup: { + modules: { + foogg: { + requires: [] + }, + boogg: { + requires: [] + } + } + } + }, + require: ['foogg', 'boogg', 'cookie', 'dom-screen'] + }); + var out = loader.resolve(true); + Assert.areSame(1, out.js.length, 'Loader generated a single URL for a single comboBase'); + + var url = out.js[0]; + Assert.isArray(url.match(/__PLACEHOLDER__/), 'Group should inherit the comboSep'); + }, + 'test inheritted comboSep with groups if comboBase is the same': function () { + var loader = new testY.Loader({ + combine: true, + groups: { + testGroup: { + comboSep: '__PLACEHOLDER__', + modules: { + foogg: { + requires: [] + }, + boogg: { + requires: [] + } + } + } + }, + require: ['foogg', 'boogg', 'cookie', 'dom-screen'] + }); + var out = loader.resolve(true); + Assert.areSame(1, out.js.length, 'Loader generated a single URL for a single comboBase'); + + var url = out.js[0]; + Assert.isNull(url.match(/__PLACEHOLDER__/), 'Group should inherit the default comboSep'); + }, + 'test different comboSep within group if comboBase is different': function () { + var loader = new testY.Loader({ + combine: true, + groups: { + testGroup: { + comboBase: 'http://secondhost.com/combo?', + comboSep: '__PLACEHOLDER__', + modules: { + foogg: { + requires: [] + }, + boogg: { + requires: [] + } + } + } + }, + require: ['foogg', 'boogg', 'cookie', 'dom-screen'] + }); + var out = loader.resolve(true); + Assert.areSame(2, out.js.length, 'Loader generated multiple URLs for a single comboBase'); + + Assert.isTrue(out.js.indexOf('http://secondhost.com/combo?3.5.0/foogg/foogg-min.js__PLACEHOLDER__3.5.0/boogg/boogg-min.js') >= 0, 'Group combo URL should be included in the result'); + Assert.isTrue(out.js.indexOf('http://yui.yahooapis.com/combo?3.5.0/cookie/cookie-min.js&3.5.0/dom-screen/dom-screen-min.js') >= 0, 'Default YUI combo URL should be included in the result'); + }, + 'test inheritted maxURLLength with groups if comboBase is the same': function () { + var loader = new testY.Loader({ + combine: true, + groups: { + testGroup: { + // Set an incredibly low maxURLLength - this should be ignored. + maxURLLength: 10, + modules: { + foogg: { + requires: [] + }, + boogg: { + requires: [] + } + } + } + }, + require: ['foogg', 'boogg', 'cookie', 'dom-screen'] + }); + var out = loader.resolve(true); + Assert.areSame(1, out.js.length, 'Loader generated a single URL for a single comboBase'); + }, test_resolve_maxurl_length: function() { var loader = new testY.Loader({ maxURLLength: 1024,