Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: patch removeEventListener to properly remove patched callbacks #158

Merged
merged 12 commits into from
Jul 24, 2020
Merged
Next Next commit
fix: patch removeEventListener to properly remove patched functions
  • Loading branch information
johnbley committed Jul 23, 2020
commit 0a2a39777584f20af4a0795acd137e923fbf563e
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export class UserInteractionPlugin extends BasePlugin<unknown> {
moduleName = this.component;
private _spansData = new WeakMap<api.Span, SpanData>();
private _zonePatched = false;
private _wrappedListeners = new WeakMap<Function, Function>();

constructor() {
super('@opentelemetry/plugin-user-interaction', VERSION);
Expand Down Expand Up @@ -193,11 +194,36 @@ export class UserInteractionPlugin extends BasePlugin<unknown> {
return listener.apply(target, args);
}
};
plugin._wrappedListeners.set(listener, patchedListener);
return original.call(this, type, patchedListener, useCapture);
};
};
}

/**
* This patches the removeEventListener of HTMLElement to handle the fact that
* we patched the original callbacks
* This is done when zone is not available
*/
private _patchRemoveEventListener() {
const plugin = this;
return (original: Function) => {
return function removeEventListenerPatched(
this: HTMLElement,
type: any,
listener: any,
useCapture: any
) {
const wrappedListener = plugin._wrappedListeners.get(listener);
if (wrappedListener) {
return original.call(this, type, wrappedListener, useCapture);
} else {
return original.call(this, type, listener, useCapture);
}
};
};
}

/**
* Patches the history api
*/
Expand Down Expand Up @@ -434,11 +460,22 @@ export class UserInteractionPlugin extends BasePlugin<unknown> {
'removing previous patch from method addEventListener'
);
}
if (isWrapped(HTMLElement.prototype.removeEventListener)) {
shimmer.unwrap(HTMLElement.prototype, 'removeEventListener');
this._logger.debug(
'removing previous patch from method removeEventListener'
);
}
shimmer.wrap(
HTMLElement.prototype,
'addEventListener',
this._patchElement()
);
shimmer.wrap(
HTMLElement.prototype,
'removeEventListener',
this._patchRemoveEventListener()
);
}

this._patchHistoryApi();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,20 @@ describe('UserInteractionPlugin', () => {
context.disable();
});

it('should not break removeEventListener', () => {
let called = false;
const listener = function () {
called = true;
};
document.body.addEventListener('testEvent', listener);
document.body.dispatchEvent(new Event('testEvent'));
assert.strictEqual(called, true);
called = false;
document.body.removeEventListener('testEvent', listener);
document.body.dispatchEvent(new Event('testEvent'));
assert.strictEqual(called, false);
});

it('should handle task without async operation', () => {
fakeInteraction();
assert.equal(exportSpy.args.length, 1, 'should export one span');
Expand Down