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

when found.configfn return false, m is null #1959

Merged
merged 2 commits into from
Sep 26, 2014

Conversation

mininice
Copy link

No description provided.

@yahoocla
Copy link

Thank you for submitting this pull request, however I do not see a valid CLA on file for you. Before we can merge this request please visit https://yahoocla.herokuapp.com/ and agree to the terms. Thanks! 😄

@okuryu
Copy link
Member

okuryu commented Sep 23, 2014

I see m has a results of the this.addModule() above a few lines. What kind of situation are you have the problem?

@mininice
Copy link
Author

function addModule has code like this, when ret === false, o = null, return null;

if (o.configFn) {
            ret = o.configFn(o);
            if (ret === false) {
                Y.log('Config function returned false for ' + name + ', skipping.', 'info', 'loader');
                delete this.moduleInfo[name];
                delete GLOBAL_ENV._renderedMods[name];
                o = null;
            }
}

@@ -2038,7 +2038,10 @@ Y.log('Undefined module: ' + mname + ', matched a pattern: ' +
temp: true
}), mname);
if (found.configFn) {
m.configFn = found.configFn;
//when found.configFn return false, m is null
if(m) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

found is always set when the module config is synthetically created based on a pattern, in which case addModule() will ALWAYS return a valid config. In other words, if found is valid, m will be valid to get a setting assigned to it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

example, I want to use patterns like this:

patterns : {
        "xxx-": {
               configFn: function(mod) {
                     //I don't want to load this mod, but don't want to use test
                     return false;
               }
       }
}

then, execute this code:

ret = o.configFn(o);
            if (ret === false) {
                Y.log('Config function returned false for ' + name + ', skipping.', 'info', 'loader');
                delete this.moduleInfo[name];
                delete GLOBAL_ENV._renderedMods[name];
                o = null;
            }

yui.js throw error, if code allow configFn return false,the after code should be compatible。

@yahoocla
Copy link

CLA is valid!

@caridy caridy closed this Sep 24, 2014
@caridy caridy reopened this Sep 25, 2014
@caridy
Copy link
Member

caridy commented Sep 25, 2014

ah, that's a very edge case, jejejeje, but I see how you might want that. I will favor this PR, but you have to change it a little bit. Just re-use the existing if statement, like if (m && found.configFn)

tripp added a commit to tripp/yui3 that referenced this pull request Sep 26, 2014
@tripp tripp merged commit 4de3991 into yui:dev-master Sep 26, 2014
@okuryu
Copy link
Member

okuryu commented Sep 29, 2014

Updated the HISTORY.md in cdc5cc3.

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

Successfully merging this pull request may close these issues.

5 participants