From b970343d654e5d1f687b40f37f7ae1227891e7f5 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Thu, 22 May 2014 13:11:50 +0800 Subject: [PATCH 1/2] The group comboBase should inherit from the default if no group setting is specified This fixes a regression caused by #1832. --- src/loader/HISTORY.md | 4 ++-- src/loader/js/loader.js | 2 +- src/loader/tests/unit/assets/loader-tests.js | 22 ++++++++++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/loader/HISTORY.md b/src/loader/HISTORY.md index 7ee92c845a8..596b02d18ca 100644 --- a/src/loader/HISTORY.md +++ b/src/loader/HISTORY.md @@ -1,10 +1,10 @@ YUI Loader Change History ========================= -@VERSION@ +3.17.2 ------ -* No changes. +* 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..7ed060115ff 100644 --- a/src/loader/js/loader.js +++ b/src/loader/js/loader.js @@ -2668,7 +2668,7 @@ Y.log('Undefined module: ' + mname + ', matched a pattern: ' + mod.root = group.root; } - comboBase = group.comboBase; + comboBase = group.comboBase || comboBase; comboSep = group.comboSep; maxURLLength = group.maxURLLength; } else { diff --git a/src/loader/tests/unit/assets/loader-tests.js b/src/loader/tests/unit/assets/loader-tests.js index b90cacf7444..8f13989bc53 100644 --- a/src/loader/tests/unit/assets/loader-tests.js +++ b/src/loader/tests/unit/assets/loader-tests.js @@ -312,6 +312,28 @@ 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: { + combine: true, + 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_resolve_maxurl_length: function() { var loader = new testY.Loader({ maxURLLength: 1024, From 1473f2e1f7953403c685afee714cdfc0cad56f33 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Thu, 22 May 2014 13:11:50 +0800 Subject: [PATCH 2/2] Groups should inherit combo-related settings from default --- src/loader/HISTORY.md | 5 + src/loader/js/loader.js | 25 +++- src/loader/tests/unit/assets/loader-tests.js | 136 +++++++++++++++++++ 3 files changed, 162 insertions(+), 4 deletions(-) diff --git a/src/loader/HISTORY.md b/src/loader/HISTORY.md index 596b02d18ca..42e9e897f04 100644 --- a/src/loader/HISTORY.md +++ b/src/loader/HISTORY.md @@ -1,6 +1,11 @@ YUI Loader Change History ========================= +3.18.0 +------ + +* Inherit group settings from base if they were not specified. ([#1838][]: @andrewnicols) + 3.17.2 ------ diff --git a/src/loader/js/loader.js b/src/loader/js/loader.js index 7ed060115ff..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 || 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 8f13989bc53..13df39660ee 100644 --- a/src/loader/tests/unit/assets/loader-tests.js +++ b/src/loader/tests/unit/assets/loader-tests.js @@ -315,9 +315,50 @@ YUI.add('loader-tests', function(Y) { '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: [] @@ -334,6 +375,101 @@ YUI.add('loader-tests', function(Y) { 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,