Update loader and yui to optionally use pathogen encoding #1950
Conversation
Add `comboHander` config to yui for allowing the use of pathogen encoding logic. Update loader to use pathogen encoding logic when `comboHandler` config is set. Add `loader-pathogen-encoder` submodule. Update build and meta files for `loader-pathogen-encoder` submodule. Changes to be committed: modified: loader/build.json modified: loader/js/loader.js modified: loader/meta/loader.json modified: yui/js/yui.js
|
CLA is valid! |
| } | ||
| //only encode if we have something to encode | ||
| if(comboSources) { |
|
looking good so far! |
|
@caridy @juandopazo |
|
|
||
| // Add the pathogen namespace to the combo base. | ||
| if (Y.config.customComboBase) { | ||
| customComboBase = Y.config.customComboBase + NAMESPACE; |
There was a problem hiding this comment.
why do we need to add p/ to customComboBase? why isn't that part of the original value? are we reusing that value for something else?
There was a problem hiding this comment.
I believe the thought there is that Y.config.customComboBase will be set by the developer and the p/ added on by the loader allowing for different urls.
Y.config.customComboBase = "http://cdn.yahooapis.com/combo/";
Y.config.customComboBase = "https://cdn.yahooapis.com/combo/";
I suppose its just as easy to set the customComboBase with the p included.
https://cdn.yahooapis.com/p/
I can easily change that.
|
I guess those are just cosmetic details +1 |
| //only encode if we have something to encode | ||
| if (hasComboModule) { | ||
| if (usePathogen) { | ||
| resolved = this._pathogenEncodeComboSources(resolved); |
There was a problem hiding this comment.
Mostly curious: why do these checks instead of just monkey-patching _encodeComboSources?
There was a problem hiding this comment.
@caridy If you click on the details for the failure it actually shows the last build passing. Not sure why its claiming failure now. Previous failures were legit and due to a sort issue which the final commit fixed.
There was a problem hiding this comment.
@juandopazo I guess the only reason not to monkey-patch _encodeComboSources is that the two methods don't share a common interface. (signature)
We'd still have to do the first check hasComboModule either way though.
…ig has been specified.
|
@caridy @juandopazo |
|
🎱 |
|
Updated the |
Replaces #1858
Need to test cross browser.