Skip to content

Commit

Permalink
fix(FEC-11038): decorator API called with incorrect this(proxy) inste…
Browse files Browse the repository at this point in the history
…ad of the activeDecorator (#546)

Issue: API method which overrides by decorator has the proxy as this, it doesn't let us use to register events on the decorator scope and other scope issues.
Solution: make a call directly to decorator API with arguments instead of the proxy.
  • Loading branch information
Yuvalke authored Mar 10, 2021
1 parent a61a710 commit 35b83ad
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 9 deletions.
14 changes: 9 additions & 5 deletions src/engines/engine-decorator.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,16 @@ class EngineDecorator extends FakeEventTarget implements IEngineDecorator {
if (prop === 'destroy') {
this._destroy();
}
if (prop === '_listeners') {
return this._listeners;
}
const activeDecorator = this._pluginDecorators.find(decorator => decorator.active);
let target;
//For events the proxy is the target - to avoid listening to engine itself
if (prop === 'addEventListener' || prop === 'removeEventListener') {
target = this;
} else {
target = activeDecorator && prop in activeDecorator ? activeDecorator : obj;
}
// $FlowFixMe
return activeDecorator && prop in activeDecorator ? activeDecorator[prop] : obj[prop];
return typeof target[prop].bind === 'function' ? target[prop].bind(target) : target[prop];
},
set: (obj, prop, value) => {
const activeDecorator = this._pluginDecorators.find(decorator => prop in decorator && decorator.active);
Expand All @@ -44,7 +48,7 @@ class EngineDecorator extends FakeEventTarget implements IEngineDecorator {

dispatchEvent(event: FakeEvent): boolean {
const activeDecorator = this._pluginDecorators.find(decorator => decorator.active);
return activeDecorator ? activeDecorator.dispatchEvent && activeDecorator.dispatchEvent(event) : super.dispatchEvent(event);
return activeDecorator && activeDecorator.dispatchEvent ? activeDecorator.dispatchEvent(event) : super.dispatchEvent(event);
}

_destroy(): void {
Expand Down
3 changes: 1 addition & 2 deletions src/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -1700,8 +1700,7 @@ export default class Player extends FakeEventTarget {
this._appendEngineEl();
} else {
if (this._engine.id === Engine.id) {
// The restoring must be done by the engine itself not by the proxy (engine decorator if exists) to make sure the engine events fired by the engine itself.
this._engine.restore.call(this._engine._engine || this._engine, source, this._config);
this._engine.restore(source, this._config);
} else {
this._engine.destroy();
this._createEngine(Engine, source);
Expand Down
4 changes: 4 additions & 0 deletions src/utils/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ const _Object = {
return item && typeof item === 'object' && !Array.isArray(item);
},

/**
* @param {any} item - The item to check if it's class
* @returns {boolean} - Whether the item is a class
*/
isClassInstance: function (item: any) {
return item && item.constructor && item.constructor.name && item.constructor.name !== 'Object';
},
Expand Down
4 changes: 2 additions & 2 deletions test/src/engines/engine-deorator.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe('EngineDecorator', () => {
player._engineDecoratorManager.createDecorators(null, null).length.should.equal(0);
});

it.skip('should decorator use the decorator instance as context of the function', done => {
it('should decorator use the decorator instance as context of the function', done => {
const decoratorManager = new EngineDecoratorManager();
const decoratorProvider = FakeDecoratorProviderActive;
decoratorManager.register(decoratorProvider);
Expand All @@ -77,7 +77,7 @@ describe('EngineDecorator', () => {
});
});

it.skip('should decorator use the engine when decorator not active', done => {
it('should decorator use the engine when decorator not active', done => {
const decoratorManager = new EngineDecoratorManager();
decoratorManager.register(FakeDecoratorProvider);
const engineDecorator = new EngineDecorator(engine, decoratorManager);
Expand Down

0 comments on commit 35b83ad

Please sign in to comment.