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] Fix group comboBase leaking into YUI modules #1832

Closed
wants to merge 3 commits into from

Conversation

juandopazo
Copy link
Member

There is an issue in Loader when YUI is configured to use more than one comboBase. In some cases YUI modules like cookie may be loaded from the group's comboBase.

@yahoocla
Copy link

CLA is valid!

@juandopazo
Copy link
Member Author

The Travis failure seems to be due to the lack of support for "^1.1.1" versions in Node 0.8.

@caridy can I get a +1?

@triptych
Copy link
Contributor

Needs a HISTORY.md entry as well please.

@ericf
Copy link
Member

ericf commented May 20, 2014

LGTM

@caridy
Copy link
Member

caridy commented May 20, 2014

+1

@unkillbob
Copy link
Contributor

This seems to have broken the case where you're not overriding the comboBase in a group.

This line:

comboBase    = group.comboBase;

overrides the comboBase with the group.comboBase even if the group.comboBase is undefined.

This is causing modules in combo loaded groups (that don't specify their own comboBase) in our environment to be requested with the URL:

<current page url>/undefined/<group root>/path/to/module.js

Changing the aforementioned line to the following resolves the issue for us:

comboBase    = group.comboBase || comboBase;

However I don't have time to put together a full pull request + tests etc right now.

andrewnicols added a commit to andrewnicols/yui3 that referenced this pull request May 22, 2014
…ng is specified

This fixes a regression caused by yui#1832.
andrewnicols added a commit to andrewnicols/yui3 that referenced this pull request May 22, 2014
…ng is specified

This fixes a regression caused by yui#1832.
andrewnicols added a commit to andrewnicols/yui3 that referenced this pull request May 22, 2014
…ng is specified

This fixes a regression caused by yui#1832.
andrewnicols added a commit to andrewnicols/yui3 that referenced this pull request May 22, 2014
…ng is specified

This fixes a regression caused by yui#1832.
andrewnicols added a commit to andrewnicols/yui3 that referenced this pull request May 22, 2014
…ng is specified

This fixes a regression caused by yui#1832.
tripp pushed a commit to tripp/yui3 that referenced this pull request May 22, 2014
…ng is specified

This fixes a regression caused by yui#1832.
@okuryu
Copy link
Member

okuryu commented May 24, 2014

This PR has been cherry-picked into the master, and includes in v3.17.1.

@okuryu okuryu closed this Jun 9, 2014
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.

7 participants