Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Loader]: Several bug fixes. #1743

Merged
merged 7 commits into from
Apr 1, 2014
Merged

[Loader]: Several bug fixes. #1743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
28e6199
[Loader]: A module's lang pack must come before the module itself.
Apr 1, 2014
5aa93f3
[loader] Test correct order with condition set to load `before`
juandopazo Feb 27, 2014
71048e5
[Loader]: Prefer `expanded` over `requires` for dependencies.
Apr 1, 2014
c55a8cf
[Loader]: Delay the processing of conditional `after` modules.
Apr 1, 2014
619ddc4
[Loader]: Strip trailing whitespace.
Apr 1, 2014
567b924
[Loader]: Convert tabs into spaces.
Apr 1, 2014
761af78
[Loader]: Avoid negative variable names.
Apr 1, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions src/loader/js/loader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2141,15 +2141,15 @@ Y.log('Undefined module: ' + mname + ', matched a pattern: ' +
* @private
*/
_sort: function() {
var name,
var name,

// Object containing module names.
required = this.required,
required = this.required,

// Keep track of whether we've visited a module.
visited = {};

// Will contain modules names, in the correct order,
// Will contain modules names, in the correct order,
// according to dependencies.
this.sorted = [];

Expand All @@ -2167,24 +2167,32 @@ Y.log('Undefined module: ' + mname + ', matched a pattern: ' +
* @param {Object} visited Keeps track of whether a module was visited.
* @method _visit
* @private
*/
*/
_visit: function (name, visited) {
var required, moduleInfo, dependency, dependencies, i, l;
var required, condition, moduleInfo, dependency, dependencies,
trigger, isAfter, i, l;

visited[name] = true;
required = this.required;
moduleInfo = this.moduleInfo[name];
condition = this.conditions[name] || {};

if (moduleInfo) {
// Recurse on each dependency of this module,
// Recurse on each dependency of this module,
// figuring out its dependencies, and so on.
dependencies = moduleInfo.requires;
dependencies = moduleInfo.expanded || moduleInfo.requires;

for (i = 0, l = dependencies.length; i < l; ++i) {
dependency = dependencies[i];

trigger = condition[dependency];

// We cannot process this dependency yet if it must
// appear after our current module.
isAfter = trigger && (!trigger.when || trigger.when === "after");

// Is this module name in the required list of modules,
// and have we not already visited it?
if (required[dependency] && !visited[dependency]) {
if (required[dependency] && !visited[dependency] && !isAfter) {
this._visit(dependency, visited);
}
}
Expand Down
57 changes: 35 additions & 22 deletions src/loader/tests/unit/assets/loader-tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,27 @@ YUI.add('loader-tests', function(Y) {
Assert.areSame('3three.js', out.js[0], 'Failed to load required module (3)');

},
'test: conditional module set to load before trigger': function () {
var loader = new Y.Loader({
modules: {
test4_one: {
fullpath: '4one.js'
},
test4_two: {
fullpath: '4two.js',
condition: {
trigger: 'test4_one',
when: 'before'
}
}
},
require: ['test4_one']
});
var out = loader.resolve(true);
Assert.areEqual(2, out.js.length, 'Wrong number of modules loaded');
Assert.areEqual('4two.js', out.js[0], 'Loaded modules in incorrect order');
Assert.areEqual('4one.js', out.js[1], 'Loaded modules in incorrect order');
},
test_css_stamp: function() {
var test = this,
links = document.getElementsByTagName('link').length + document.getElementsByTagName('style').length;
Expand Down Expand Up @@ -1070,10 +1091,8 @@ YUI.add('loader-tests', function(Y) {
});

var out = loader.resolve(true);
// The order of `out.js`'s elements do not matter in this case,
// as both modules do not depend on one another.
Assert.areSame('sub1/lang/subsub2.js', out.js[1], 'Failed to combine submodule with module path for LANG JS');
Assert.areSame('sub1/subsub2.js', out.js[0], 'Failed to combine submodule with module path JS');
Assert.areSame('sub1/lang/subsub2.js', out.js[0], 'Failed to combine submodule with module path for LANG JS');
Assert.areSame('sub1/subsub2.js', out.js[1], 'Failed to combine submodule with module path JS');
Assert.areSame('sub1/assets/skins/sam/subsub2.css', out.css[0], 'Failed to combine submodule with module path CSS');
Assert.areEqual(1, out.css.length, 'Failed to skin submodule');
},
Expand Down Expand Up @@ -1107,12 +1126,10 @@ YUI.add('loader-tests', function(Y) {

var out = loader.resolve(true);

// `plug1/subplug2` and `plug1/lang/subplug2` depend strictly on `plug1/subplug1`.
// So, the only requirement is that `plug1/subplug1` must come *before* both `plug1/subplug2` and `plug1/lang/subplug2`.
Assert.areSame('plug1/lang/subplug2.js', out.js[2], 'Failed to combine plugin with module path LANG JS');
Assert.areSame('plug1/lang/subplug1.js', out.js[3], 'Failed to combine plugin with module path LANG JS');
Assert.areSame('plug1/subplug1.js', out.js[0], 'Failed to combine plugin with module path JS');
Assert.areSame('plug1/subplug2.js', out.js[1], 'Failed to combine plugin with module path JS');
Assert.areSame('plug1/lang/subplug2.js', out.js[0], 'Failed to combine plugin with module path LANG JS');
Assert.areSame('plug1/lang/subplug1.js', out.js[1], 'Failed to combine plugin with module path LANG JS');
Assert.areSame('plug1/subplug1.js', out.js[2], 'Failed to combine plugin with module path JS');
Assert.areSame('plug1/subplug2.js', out.js[3], 'Failed to combine plugin with module path JS');
Assert.areSame('plug1/assets/skins/sam/subplug1.css', out.css[0], 'Failed to combine plugin with module path CSS');
Assert.areSame('plug1/assets/skins/sam/subplug2.css', out.css[1], 'Failed to combine plugin with module path CSS');
Assert.areEqual(2, out.css.length, 'Failed to skin plugins');
Expand Down Expand Up @@ -1595,9 +1612,8 @@ YUI.add('loader-tests', function(Y) {
require: ['my-module']
});
var out = loader.resolve(true);
// The only requirement here is `lang` must come *after* `mod`.
var mod = out.js[0];
var lang = out.js[4];
var mod = out.js[7];
var lang = out.js[6];
Assert.areEqual('scripts/my-module.js', mod, 'Failed to resolve module');
Assert.areEqual('scripts/my-module/lang/my-module_fr.js', lang, 'Failed to resolve local lang file');
},
Expand All @@ -1620,9 +1636,8 @@ YUI.add('loader-tests', function(Y) {
require: ['my-module-group']
});
var out = loader.resolve(true);
// The only requirement here is `lang` must come *after* `mod`.
var mod = out.js[0];
var lang = out.js[4];
var mod = out.js[7];
var lang = out.js[6];
Assert.areEqual('scripts/my-module.js', mod, 'Failed to resolve module');
Assert.areEqual('scripts/my-module-group/lang/my-module-group_fr.js', lang, 'Failed to resolve local lang file');
},
Expand Down Expand Up @@ -1698,15 +1713,13 @@ YUI.add('loader-tests', function(Y) {
});

Assert.areEqual(6, other.length, 'Failed to resolve all modules and languages');
// The only requirement here is that the `lang/*` versions
// of each module must appear *after* the original version (whatever * is).
var expected = [
"./root-lang-fail/root-lang-fail.js",
"./root-lang-win/root-lang-win.js",
"./de-lang/de-lang.js",
"./root-lang-fail/lang/root-lang-fail.js",
"./root-lang-fail/root-lang-fail.js",
"./root-lang-win/lang/root-lang-win.js",
"./de-lang/lang/de-lang.js"
"./root-lang-win/root-lang-win.js",
"./de-lang/lang/de-lang.js",
"./de-lang/de-lang.js"
];
ArrayAssert.itemsAreEqual(expected, other, 'Failed to resolve the proper modules');

Expand Down