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.

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

[Loader] Sorting modules incorrectly when YUI.GlobalConfig.groups is configurated #639

Open
springuper opened this issue Apr 19, 2013 · 3 comments
Labels
Milestone

Comments

@springuper
Copy link

When we upgrade our site from 3.5.1 to 3.8.0, a strange error occurs again and again. The code is very simple:

var nlAddCart = Y.all('.add-cart');
nlAddCart.on('click', function (e) {
    e.halt();
    // do sth
});

There are two items in nlAddCart, but neither acts as we wish when a click event happens.

I spend one day to find out the reason, and finally get it:

Step 1: YUI.GlobalConfig is initialized with a object o, which has a groups property to contain all our modules:

YUI.GlobalConfig = {
    // ...
    groups: {
        fecore: {
            'mt-base': {
                requires: ['yui', 'node-base'],
                path: //...
            }
        }
    }
}

Step 2: when a new YUI instance Y is created, o.groups is mixed to Y.config

Step 3: when Y.use is called for the first time, a new Y.Loader instance, named loader, is created with Y.config. loader._config is automatically called in the construction process, and then loader.addGroup, and then loader.addModule,

// https://github.com/yui/yui3/blob/v3.8.0/build/yui/yui.js#L6653
this.moduleInfo[name] = o;

all module meta on YUI.GlobalConfig.groups is assigned to loader.moduleInfo.

Step 4: in the loader._explode method (explode all modules directly or indirectly required by used modules, it is called by loader.calculate in Y.use), an important method loader.getRequires is called to collect all modules directly or indirectly required by current module, and assign this collection result to the module's property expanded/expanded_map. Therefore, the meta in YUI.GlobalConfig is changed globally. To optimize performance, loader.getRequires detects some features to avoid needless computing:

// https://github.com/yui/yui3/blob/v3.8.0/build/yui/yui.js#L7012
reparse = !((!this.lang || mod.langCache === this.lang) && (mod.skinCache === this.skin.defaultSkin));

if (mod.expanded && !reparse) {
    return mod.expanded;
}

Step 5: in the loader._sort method called after loader._explode, a comparison method loader._require(mod1, mod2) is called for each pair of modules to determine which should be put ahead. The expanded/expanded_map property of a module plays a big role.

Step 6: in the first YUI.use('mt-base') statement, all modules required by 'mt-base' are processed by loader.getRequire method and consequently gets the expanded/expanded_map property. But when YUI.use('mt-base') runs in the second time, as 'mt-base' already has expanded, skinCache, and langCache properties, loader.getRequire returns expanded result directly according to Step 4. Therefore, every YUI built-in modules required by 'mt-base' havs no expanded property, finally, loader._sort method computes a wrong sequence. In this example, the order is:

["yui", "node-base", "event-base", "event-custom-base", "oop", "yui-base", "node-core", "dom-core", "features", "selector", "selector-native", "dom-base", "mt-base"]

This causes Y._attach process 'event-base' before 'dom-core' and 'dom-base', and further results in shouldIterate method gets a wrong answer. So, nlAddCart.on fails.

In summary, this problem is caused by two points:

  • meta info on YUI.GlobalConfig is used and modified by loader.getRequires
  • the optimization of loader.getRequires results in some YUI builtin modules missing expanded/expanded_map property, and finally causes loader._sort gives a wrong module sequence.

The full code:

YUI.GlobalConfig = {
    root: '...',
    combine: true,
    comboBase: '...',
    groups: {
        fecore: {
            'mt-base': {
                requires: ['yui', 'node-base'],
                path: '...'
            }
        }
    }
};

YUI().use('mt-base', function (Y) {
    // do sth
});
YUI().use('mt-base', function (Y) {
    var nlAddCart = Y.all('.add-cart');
    nlAddCart.on('click', function (e) {
        e.halt();
        // do sth
    });
});
@caridy
Copy link
Member

caridy commented Apr 19, 2013

This might be related to issue #559

@springuper
Copy link
Author

No one should deal with this bug? We have hacked through our YUI().use wrapper, but it's really strong desired for YUI team to repair this problem.

@ezequiel ezequiel self-assigned this Mar 8, 2014
@ezequiel ezequiel added this to the Sprint 13 milestone Mar 10, 2014
@ezequiel
Copy link
Contributor

@springuper,

does this problem still happen on yui 3.17.2? we recently rewrote loader's _sort algorithm, so your problem may have been inadvertently fixed. it no longer uses expanded, expanded_map, or _requires

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants