Event Timeline
Here's the approach that we've settled on:
- Campaign-associated Mixins should be RL modules
- Instead of carrying JS, choiceData will just have the names of any campaign-associated mixin modules needed and the parameters to be used
- bannerLoader will call mw.loader.load for the modules needed. The first time a user is targeted by a campaign that needs any given module, this will add a request to the process, but on subsequent page views, the module should be retrieved from the browser's localStorage. So, no more extra requests after the first one.
Non-blocking issues and considerations:
- Some refactoring of the JS currently in-banner will be needed.
- We'll try to consolidate bits of JS that are currently used together. It doesn't make sense to have very small modules or to spam the RL module registry.
- We should only mixinize bits of JS that aren't expected to change too frequently, and make sure that the Mixins' parameters can handle most forseeable tuning needs. Changes in the actual JS will require a deploy.
The other option that was considered was: add commonly used functions to bannerController.lib and include actual small JS snippets in choiceData. However, it looks like the above approach will be more performant overall.
@Ejegg and @Krinkle have looked over this plan (though they shouldn't get any blame if the plane runs ahsore).
That sounds great! One small question though, since the RL modules are aggressively cached as you mention, why bother with the extra complexity of passing the module name and then loading explicitly, rather than just loading the module on the first pageview along with the others, if one of the campaigns targeting the viewer requires it?
Hmmm! WRT loading right away if any campaign choices require it: that would be great—if we can do it! Whether or not it's possible I think will depend on the delicacies of RL module caching and dependency resolution. Potentially we could make CNBannerChoiceDataResourceLoaderModule add dependencies based on the choices available... maybe... :)
I'd be mainly interested in that route if it could save us the extra request the first time a mixin module is needed. WRT the complexity of passing the module name, I'm not too worried, since we have to pass the name of the mixin required, in any case. True, the dynamic load may bring in some complexity for coordinating all the pieces of JS to run in the right order, but I think that'd be OK if it's the best option performance-wise.
Yuup! Unclosing and backwardsing to PR.
It looks like we could override getDependencies() in CNBannerChoiceDataResourceLoaderModule. That's called (eventually) via ResourceLoaderStartUpModule::getScript(). So in getDependencies() we could check the banner choices and add required mixin modules as dependencies. So long as caching of the startup module is long enough that we don't get very frequent calls, but not longer than current caching for CNBannerChoiceDataResourceLoaderModule, I think we can do it. :) (Just trying to grok the varnish setup again for that...)
Right, I hadn't thought this through, now I see why you're planning to load the extra RL modules manually. Still, the getDependencies thing, if it works, might pull the extra modules in slightly earlier (second or third initial RL rounds) than if we're putting this logic in the banner controller.
It's probably worthwhile to investigate, simply because the first and second impression viewers are such a delectable target :)
OK! I don't see anything in the varnish config that would make the startup RL module caching different from any others. Let's try this first!
Change 198267 had a related patch set uploaded (by AndyRussG):
WIP Campaign-associated mixins: schema update