YUI 3

Ticket #2531587 (closed defect)

Reporter


J. Shirley 		Opened: 12/13/11
Last modified: 05/10/12
Status: closed
Type: defect
Resolution: fixed

Owner


Dav Glass 		Target Release: 3.5.0
Priority: P2 (high)
Summary: Loader will not load custom modules if combine: true
Description:

Loader will load dependencies, but not any custom module (regardless if it is in a group or a top level module) unless combine: false (or the custom module is also loaded through a combo service).

I have a page setup with 3.5.0pr1 and 3.4.1 seeds, using the same code as presented that shows the issue clearly.
Type: defect Observed in Version: 3.5.0pr1
Component: Loader Severity: S3 (normal)
Assigned To: Dav Glass Target Release: 3.5.0
Location: Library Code Priority: P2 (high)
Tags: Relates To:
Browsers: N/A
URL: http://j.shirley.im/tech/yui/issues/loader-test.html
Test Information:

Attachments

Attachment #1: sample.html (download)

Change History

Dav Glass
Posted: 12/13/11
  • location changed to Library Code
  • milestone changed to 3.5.0
  • priority changed to P2 (high)
  • severity changed from S2 (high) to S3 (normal)
  • status changed from new to accepted

Dav Glass
Posted: 12/13/11
  • estimated changed from 0 to 0.5
  • remaining changed from 0 to 0.5
  • sprint changed to sprint 2

Dav Glass
Posted: 12/14/11

Ticket #2531597 was marked as a duplicate of this ticket.

J. Shirley

YUI Contributor
Posted: 12/14/11

I've found an additional issue, but not sure exactly how related it is (so it may be a new ticket).

If I take a working configuration and try to override the gallery, the gallery then will fail to load (this also was working in 3.4.1). In the example from the URL, if I simply add in something like the gallery definition below, no gallery modules will load:



            gallery: {

                base      : 'http://yui.ed.je/combo?gallery-2011.12.05-14-17/build/',

                comboBase : 'http://yui.ed.je/combo?',

                root      : 'gallery-2011.12.05-14-17/build/',

                patterns: {

                    "gallery-"    : {},

                    "gallerycss-" : { type : "css" }

                }

            },

I do this for testing my own gallery modules prior to a pull, since we have a QA cycle I can throw at them. I can't find any combination of combine: values that cause it to work. I can, however, list the individual gallery modules and the path (but that's not an option, as we heavily rely on the gallery).

Thanks.

J. Shirley

YUI Contributor
Posted: 12/14/11

I've found that I must also have a top level `gallery` tag, which if I have that (depending upon configuration) may throw an error:



this.groups.gallery.update(val);

combo:6021

TypeError: 'undefined' is not a function (evaluating 'this.groups.gallery.update(val)')

(Using yui.js, L6021 of combo).

Dav Glass
Posted: 12/14/11

J, can you please file that second issue as a separate ticket so I can track it. They are two different issues and need to be tracked as two.

Thanks

Dav Glass
Posted: 12/14/11

Can you test against this patch before I push it to CDN for use?
https://raw.github.com/gist/1478138/ff7f206c79868ecfcb91f1febd4a04693841beba/gistfile1.js

Simply add it directly after your seed file and it should "just work"

J. Shirley

YUI Contributor
Posted: 12/14/11

I'll post the second ticket shortly, for now that doesn't seem to work:

I modified the other fiddle, I think this is what you meant should work:

http://jsfiddle.net/cZWjW/3/

Dav Glass
Posted: 12/14/11

Um, it helps if you close the script tag :)

http://jsfiddle.net/davglass/PrqsC/

J. Shirley

YUI Contributor
Posted: 12/14/11

Shocking! Thanks :)

Dav Glass
Posted: 12/15/11

Did that patch also fix your gallery loading issue as well?

J. Shirley

YUI Contributor
Posted: 12/15/11

Yeah -- that ended up being the same thing (the other issue I saw was also present in 3.4.1 and trying to figure out exactly what's goin on).

Thanks!

Jamison
Posted: 12/21/11
It tried adding the patch script as indicated and my custom module would not load, but with or without the patch included it works fine with 3.4.1. This was also the case when all combo loading is turned off, in the Firebug console I see a "No modules resolved.." information message from the loader, and then the errors start. My YUI call is a bit convoluted because there is dynamic module generation going on, but the code looks like this:
<code>
var mods = {}, c = PWC.buildYconf();
mods['pwc-form-' + modName] = {fullpath: '/app/js/' + modName};
c.modules = mods;
YUI(c).use('node', 'pwc-form-' + modName, function (Y) {
...
}

</code>
This is the result both when I use my local combo loader or YUI off the CDN.

Jamison
Posted: 12/21/11

Sorry, that code should be:



var mods = {},

 c = {combine: false};

 mods['pwc-form-' + modName] = {fullpath: '/app/js/' + modName};

 c.modules = mods;

 YUI(c).use('node', 'pwc-form-' + modName, function (Y) {

 ...

 });

Jamison
Posted: 12/21/11

I just added an attachment, sample.html, from my testing trying to isolate the problem, with combine: true the patch will work on that particular page (though not with my other code) but the attached file is configured with combine: false and the module 'node' does not get attached to Y, resulting in an error. (You will see a "Loaded" panel in the middle of the page if it works.)

Dav Glass
Posted: 01/3/12
  • status changed from accepted to infoneeded

@Jamison Can you try this patch?
https://raw.github.com/gist/1478138/34daf89b5c809af91fa8520e5f183db095f2e734/gistfile1.js

Dav Glass
Posted: 01/3/12

Patch for #2531587 updated
View Commit: c0f8d099377c6de65135439e75cf1d64649f3868

Jamison
Posted: 01/3/12
  • status changed from infoneeded to assigned

Jamison
Posted: 01/3/12

The updated gist does fix the sample.html that I attached to this ticket, but my app is still breaking if I update to 3.5pr1 + that gist from 3.4.1. (I have tested with my locally hosted 3.4.1, 3.4.1 on the CDN and 3.5.0pr1 on the CDN, only 3.5.0pr1 on the CDN does not work).

This code does not seem to trigger any request to the URL "/app/js/xlogin" on the hosting server:



<script type="text/javascript">

YUI({modules: { 'xlogin': {fullpath: '/app/js/xlogin'
).use('xlogin', function() {
doSomething();
});
</script>

This tells me that the modules are never loaded but I do not quite understand why the request to "use" a module that is not available does not cause an immediate error given that no attempt is being made to add the module.
Is there a syntax change or something that is causing the modules config to be incorrect that I am not finding?

I'll try to put something stand-alone together that can demonstrate the issue better shortly.

Jamison
Posted: 01/3/12

Sorry, my code had a triple-brace in it.. making the wikisyntax break.

I have attached an updated copy of sample.html. When I load this page no attempt is made to access the URL the module to load is supposed to be located at but the other modules load just fine and the script included runs. I would expect an attempt to be made to load the module and for that attempt to fail and the use callback to never get executed.

Dav Glass
Posted: 01/4/12
  • status changed from assigned to infoneeded

I'm not seeing that behavior in master or using PR1 with the latest loader:

https://img.skitch.com/20120104-pqfgee7rxqc6xyur53jxrtkhwm.jpg

Using this version of Loader:
https://github.com/davglass/yui3/blob/master/build/loader/loader-min.js

And this example:
https://github.com/davglass/yui3/blob/master/sandbox/loader/bug_2531587.html

Jamison
Posted: 01/4/12

I did not work for me if I updated my attached page to use your loader and pr1 the same as what is in your example. What I get now is the loader actually generating a bad combo request, I get a 400 back from the yahoo server. It turns out that I can avoid that problem if I remove the references to the overlay and panel modules. After some fooling around I found the minimal request that can reproduce the combo loader problem for pr1:
http://yui.yahooapis.com/combo?3.5.0pr1/build/attribute-core/attribute-core-min.js
But that is a different bug than the one we are looking at.

Now I changed my app, but there it is still not working. I will trim my app's generated page down and attach anther page the reproduces the problem shortly.

Jamison
Posted: 01/4/12
  • status changed from infoneeded to assigned

Jamison
Posted: 01/4/12

I updated the attached sample.html, the ticket does not seem to indicate that when I actually update it. (Next time I will rename it, I expected it to be added not updated.)

Dav Glass
Posted: 01/4/12

Ref #2531587 - Added better error handling for 404's

The complete method now handles failures and timeouts
as well as success.
View Commit: f05ba2b61f71e5e5da569db5bcb87f33bea42140

Dav Glass
Posted: 01/4/12
  • status changed from assigned to infoneeded

@Jamison --

I tried your example again and it worked for me:
https://img.skitch.com/20120104-jen5uf6k5q4pre1ea64scqn4wx.jpg

However, the issue may be in your module data. Can you give me a stripped down version of what `/app/js/xlogin` looks like so I can see if it's loading or not.

Jamison
Posted: 01/5/12
  • status changed from infoneeded to assigned

The module xlogin does not really exist, I wrote up the sample file to demonstrate the problem with a module that does not exist on purpose, using the Firefox Web Console this is what I see if I download the attached file and open it in Firefox 8.0.1:



[03:36:45.309] GET http://yui.yahooapis.com/3.5.0pr1/build/yui/yui-debug.js [HTTP/1.1 304 Not Modified 172ms]

[03:36:45.323] GET http://_Sraw.github.com/davglass/yui3/master/build/loader/loader.js [HTTP/1.1 304 Not Modified 88ms]

[03:36:45.562] yui: Modules missing: xlogin, 1 @ http://yui.yahooapis.com/3.5.0pr1/build/yui/yui-debug.js:5122

[03:36:45.566] yui: Using Loader @ http://yui.yahooapis.com/3.5.0pr1/build/yui/yui-debug.js:5122

[03:36:45.599] GET http://yui.yahooapis.com/combo?3.5.0pr1/build/oop/oop-min.js&amp;3.5.0pr1/build/event-custom-base/event-custom-base-min.js&amp;3.5.0pr1/build/event-custom-complex/event-custom-complex-min.js&amp;3.5.0pr1/build/intl/intl-min.js [HTTP/1.1 200 OK 238ms]

[03:36:45.843] SUCCESS OUTSIDE:  ({0:{msg:"success", data:["xlogin"], success:true, failed:[], skipped:{
) @ file:///C:/Users/Jamison/AppData/Local/Temp/sample.html:11

As I think you can see, there is no attempt to load xlogin. Now, if that is because I am loading the file off disk instead of a server and there is a security issue I'd understand that - but there is no error. If I pop the file up on my local web server, I get the same result:



[03:50:38.803] GET http://192.168.137.181:8765/sample.html [HTTP/1.1 200 OK 73ms]

[03:52:39.915] GET http://yui.yahooapis.com/3.5.0pr1/build/yui/yui-debug.js [HTTP/1.1 304 Not Modified 163ms]

[03:50:38.941] GET http://_Sraw.github.com/davglass/yui3/master/build/loader/loader.js [HTTP/1.1 304 Not Modified 347ms]

[03:50:39.317] yui: Modules missing: xlogin, 1 @ http://yui.yahooapis.com/3.5.0pr1/build/yui/yui-debug.js:5122

[03:50:39.324] yui: Using Loader @ http://yui.yahooapis.com/3.5.0pr1/build/yui/yui-debug.js:5122

[03:50:39.390] SUCCESS OUTSIDE:  ({0:{msg:"success", data:["xlogin"], success:true, failed:[], skipped:{
) @ http://192.168.137.181:8765/sample.html:11

Jamison
Posted: 01/5/12

Too bad you can't preview a comment before posting it.. let me try that again.



<nowiki>

[03:59:41.103] GET http://192.168.137.181:8765/sample.html [HTTP/1.1 304 Not Modified 48ms]

[03:59:41.158] GET http://yui.yahooapis.com/3.5.0pr1/build/yui/yui-debug.js [HTTP/1.1 304 Not Modified 161ms]

[03:59:41.171] GET http://_Sraw.github.com/davglass/yui3/master/build/loader/loader.js [HTTP/1.1 304 Not Modified 347ms]

[03:59:41.576] yui: Modules missing: xlogin, 1 @ http://yui.yahooapis.com/3.5.0pr1/build/yui/yui-debug.js:5122

[03:59:41.580] yui: Using Loader @ http://yui.yahooapis.com/3.5.0pr1/build/yui/yui-debug.js:5122

[03:59:41.612] GET http://yui.yahooapis.com/combo?3.5.0pr1/build/oop/oop-min.js&amp;3.5.0pr1/build/event-custom-base/event-custom-base-min.js&amp;3.5.0pr1/build/event-custom-complex/event-custom-complex-min.js&amp;3.5.0pr1/build/intl/intl-min.js [HTTP/1.1 200 OK 169ms]

[03:59:41.786] SUCCESS OUTSIDE:  ({0:{msg:"success", data:["xlogin"], success:true, failed:[], skipped:{
) @ http://192.168.137.181:8765/sample.html:11
</nowiki>

Jamison
Posted: 01/5/12

Sorry, one more time!



[03:59:41.103] GET http://192.168.137.181:8765/sample.html [HTTP/1.1 304 Not Modified 48ms]

[03:59:41.158] GET http://yui.yahooapis.com/3.5.0pr1/build/yui/yui-debug.js [HTTP/1.1 304 Not Modified 161ms]

[03:59:41.171] GET http://_Sraw.github.com/davglass/yui3/master/build/loader/loader.js [HTTP/1.1 304 Not Modified 347ms]

[03:59:41.576] yui: Modules missing: xlogin, 1 @ http://yui.yahooapis.com/3.5.0pr1/build/yui/yui-debug.js:5122

[03:59:41.580] yui: Using Loader @ http://yui.yahooapis.com/3.5.0pr1/build/yui/yui-debug.js:5122

[03:59:41.612] GET http://yui.yahooapis.com/combo?3.5.0pr1/build/oop/oop-min.js&amp;3.5.0pr1/build/event-custom-base/event-custom-base-min.js&amp;3.5.0pr1/build/event-custom-complex/event-custom-complex-min.js&amp;3.5.0pr1/build/intl/intl-min.js [HTTP/1.1 200 OK 169ms]

[03:59:41.786] SUCCESS OUTSIDE:  ({0:{msg:"success", data:["xlogin"], success:true, failed:[], skipped:{ } } } ) @ http://192.168.137.181:8765/sample.html:11

Dav Glass
Posted: 01/5/12
  • status changed from assigned to accepted

I just pushed an update that will fix this, the diff between what you had and what I has was I had combine set to false. Once I set it back to true, it broke for me too..

Dav Glass
Posted: 01/5/12

Ref #2531587 - Missed a module not defined inside a group

The resolve method does two passes on modules, once for
comboed, them once to catch the "other" defined modules.
I forgot to catch a module defined under {modules{}} and
add it to the list to process next.
View Commit: b04bd1290994ae05c7e6ec6c19b05fb733041989

Dav Glass
Posted: 01/5/12
  • status changed from accepted to checkedin

Dav Glass
Posted: 01/5/12

Fixes #2531626, #2531637, ref #2531587

Added support for comboSep and maxURLLength for groups.
Removed the arbitrary Math.min on the MAX_URL_LENGTH
default value. Dropped the default from 2048 to 1024.
Fixed issues with resolve not splitting URL's correctly.
View Commit: 98a720969945c9ca59a1fdeb40d248e8137ebb16

Dav Glass
Posted: 01/5/12

Refs #2531626, #2531637, #2531587 - Added more loader tests

Loader tests for comboSep, maxURLLength in groups.
Modified Loader tests for better assertion reporting
and updated URL counts for the new, lower maxURLLength.
View Commit: 69232557ff717ee3f764bfcac887ba6182d483ef

Jamison
Posted: 01/5/12

Is this the loader I should be using to re-test?

http://_Sraw.github.com/davglass/yui3/master/build/loader/loader.js

If so, then I get a new error:



[16:55:31.141] mname.indexOf is not a function @ http://_Sraw.github.com/davglass/yui3/master/build/loader/loader.js:1801

(Firefox 8.0.1)

If not, what should I use to re-test?

Dav Glass
Posted: 01/6/12
  • milestone changed from 3.5.0
  • status changed from checkedin to reopened

@Jamison - Same test page? I don't see that error & I can't replicate it. I can't even see how that error would be thrown, please add a repro case and I will get it patched up.

Dav Glass
Posted: 01/6/12
  • milestone changed to 3.5.0

Dav Glass
Posted: 01/6/12
  • status changed from reopened to accepted

Jamison
Posted: 01/6/12

The attached file reproduces for me, I just clicked on the download link ( http://yuilibrary.com/projects/yui3/download/2531587/sample.html ) and selected to open it with firefox, opened the web console and reloaded the page, here is the log:



[14:00:37.048] file:///C:/Users/Jamison/AppData/Local/Temp/sample-1.html

[14:00:37.123] GET http://yui.yahooapis.com/3.5.0pr1/build/yui/yui-debug.js [HTTP/1.1 304 Not Modified 212ms]

[14:00:37.142] GET http://_Sraw.github.com/davglass/yui3/master/build/loader/loader.js [HTTP/1.1 304 Not Modified 110ms]

[14:00:37.421] yui: Modules missing: xlogin, 1 @ http://yui.yahooapis.com/3.5.0pr1/build/yui/yui-debug.js:5122

[14:00:37.425] yui: Using Loader @ http://yui.yahooapis.com/3.5.0pr1/build/yui/yui-debug.js:5122

[14:00:37.452] mname.indexOf is not a function @ http://_Sraw.github.com/davglass/yui3/master/build/loader/loader.js:1799

That is with Firefox 8.0.1 and I can reproduce it using an old 3.6.24 build too.

Dav Glass
Posted: 01/6/12

I'm not sure why that fix didn't get into the commit and pushed, but I just put the proper fix in & pushed to my repo as well as our build server. (long week)

Jamison
Posted: 01/6/12

I quick run through firebug and it shows me that the mname param in p.test() is an object (the metadata from the module list) not the just name of the module. If the parameter is correct then the test method should be:



p.test = function(mname, pname) {

 return (mname.name.indexOf(pname) > -1);

};

(Although I would say that with the caveat that I do not understand the purpose of this test method -- it seems to be checking if it is a gallery module or not.)
The mname parameter to test comes from the mname parameter from getModule() which itself comes from being pushed onto the *singles* var in resolve() when it is not a combo. It seems possible that the parameter is wrong and it should just be the name, in that case I guess the change would be here (line 2340):


if (!group.combine) {

m.combine = false;

//This is not a combo module, skip it and load it singly later.

singles.push(s[i].name);

continue;

}

From what I can tell this is the place to change, if I set a breakpoint in getModule for all the built-in YUI modules it is just the name that is passed in.

Dav Glass
Posted: 01/6/12
  • resolution changed to fixed
  • status changed from accepted to checkedin

Fixes #2531587 - Not sure how this commit got missed, but adding it back in now.
View Commit: 83eb52bcdbf6f2d3b7abafd9c7486d64c62f642b

Dav Glass
Posted: 01/6/12

Yeah, that was fixed yesterday, not sure why that was reverted between then and now. I just added it back in and it works for me now.

Jamison
Posted: 01/6/12

Sorry, now it doesn't try to load xlogin anymore.. :(

Dav Glass
Posted: 01/6/12

You need the seed too:

https://gist.github.com/1572589

Jenny Donnelly

YUI Developer
Posted: 05/10/12
  • status changed from checkedin to closed

checkedin -> closed

© 2006-2013 Yahoo! Inc. All rights reserved.
All code on this site is licensed under the BSD License unless stated otherwise.
About This Site · Security Contact Info