Do not load external js if renderer defined on adUnit #3284
Conversation
| // we expect to load a renderer url once only so cache the request to load script | ||
| loadScript(url, this.callback, true); | ||
| } else { | ||
| utils.logWarn(`External Js not loaded by Renderer since renderer url and callback is already defined on adUnit ${adUnitCode}`); |
There was a problem hiding this comment.
Does this need to be a warning?
Given the nature of the setup (ie defining a renderer in the adunit), this seems like a logInfo type message since the user isn't doing anything wrong and the code is handling this scenario as expected. Unless I'm missing something?
There was a problem hiding this comment.
Here the user has set that renderer defined on adUnit will render the bid but the demand partners they are working with are using different js to render.
| @@ -246,6 +248,7 @@ function newBid(serverBid, rtbBid, bidderRequest) { | |||
| currency: 'USD', | |||
| netRevenue: true, | |||
| ttl: 300, | |||
| adUnitCode: bidRequest.adUnitCode, | |||
There was a problem hiding this comment.
Why is this needed on the bid?
There was a problem hiding this comment.
Code in master was pointing to bid.adUnitCode which was undefined until now. Hence i added it.
Prebid.js/modules/appnexusBidAdapter.js
Line 276 in a98a349
* do not load external js if renderer defined on adUnit * Added unit test
Do not merge
Type of change
Description of change
Fixes #2805, Delaying loading of external js for winning bid will be done in a separate PR. This one just fixes a bug.
If renderer is defined on adUnit level, Renderer module will not load external js returned by bid. As of now 12 adapters are using Renderer. We need to update those if this approach is fine.