Skip to content

Commit

Permalink
fix(Renderer): load script only on render (#5235)
Browse files Browse the repository at this point in the history
  • Loading branch information
jeremiegirault authored May 26, 2020
1 parent 88033c0 commit 0b52d6a
Showing 2 changed files with 23 additions and 8 deletions.
23 changes: 16 additions & 7 deletions src/Renderer.js
Original file line number Diff line number Diff line change
@@ -37,12 +37,21 @@ export function Renderer(options) {
this.process();
});

if (!isRendererDefinedOnAdUnit(adUnitCode)) {
// we expect to load a renderer url once only so cache the request to load script
loadExternalScript(url, moduleCode, this.callback);
} else {
utils.logWarn(`External Js not loaded by Renderer since renderer url and callback is already defined on adUnit ${adUnitCode}`);
}
// use a function, not an arrow, in order to be able to pass "arguments" through
this.render = function () {
if (!isRendererDefinedOnAdUnit(adUnitCode)) {
// we expect to load a renderer url once only so cache the request to load script
loadExternalScript(url, moduleCode, this.callback);
} else {
utils.logWarn(`External Js not loaded by Renderer since renderer url and callback is already defined on adUnit ${adUnitCode}`);
}

if (this._render) {
this._render.apply(this, arguments) // _render is expected to use push as appropriate
} else {
utils.logWarn(`No render function was provided, please use .setRender on the renderer`);
}
}.bind(this) // bind the function to this object to avoid 'this' errors
}

Renderer.install = function({ url, config, id, callback, loaded, adUnitCode }) {
@@ -54,7 +63,7 @@ Renderer.prototype.getConfig = function() {
};

Renderer.prototype.setRender = function(fn) {
this.render = fn;
this._render = fn;
};

Renderer.prototype.setEventHandlers = function(handlers) {
8 changes: 7 additions & 1 deletion test/spec/renderer_spec.js
Original file line number Diff line number Diff line change
@@ -126,10 +126,13 @@ describe('Renderer', function () {
id: 1,
adUnitCode: 'video1'
});
testRenderer.setRender(() => {})

testRenderer.render()
expect(utilsSpy.callCount).to.equal(1);
});

it('should call loadExternalScript() for script not defined on adUnit', function() {
it('should call loadExternalScript() for script not defined on adUnit, only when .render() is called', function() {
$$PREBID_GLOBAL$$.adUnits = [{
code: 'video1',
renderer: {
@@ -143,6 +146,9 @@ describe('Renderer', function () {
id: 1,
adUnitCode: undefined
});
expect(loadExternalScript.called).to.be.false;

testRenderer.render()
expect(loadExternalScript.called).to.be.true;
});
});

0 comments on commit 0b52d6a

Please sign in to comment.