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

feat: unify the signatures of bind and with #2247

Merged
merged 10 commits into from
Jun 30, 2021
1 change: 1 addition & 0 deletions .github/workflows/unit-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ jobs:
- name: Install and Build (cache hit) 🔧
if: steps.cache.outputs.cache-hit == 'true'
run: |
chown -R 1001:121 "/github/home/.npm" || true # fix npm cache permissions for npm v7 if cache exists
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Node 16 error was fixed in another PR please remove this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed it!

npm ci --ignore-scripts
npx lerna bootstrap
npm run compile
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,16 @@ export abstract class AbstractAsyncHooksContextManager
*/
bind<T>(context: Context, target: T): T {
if (target instanceof EventEmitter) {
return this._bindEventEmitter(target, context);
return this._bindEventEmitter(context, target);
}

if (typeof target === 'function') {
return this._bindFunction(target, context);
return this._bindFunction(context, target);
}
return target;
}

private _bindFunction<T extends Function>(target: T, context: Context): T {
private _bindFunction<T extends Function>(context: Context, target: T): T {
const manager = this;
const contextWrapper = function (this: never, ...args: unknown[]) {
return manager.with(context, () => target.apply(this, args));
Expand All @@ -91,12 +91,12 @@ export abstract class AbstractAsyncHooksContextManager
* By default, EventEmitter call their callback with their context, which we do
* not want, instead we will bind a specific context to all callbacks that
* go through it.
* @param ee EventEmitter an instance of EventEmitter to patch
* @param context the context we want to bind
* @param ee EventEmitter an instance of EventEmitter to patch
*/
private _bindEventEmitter<T extends EventEmitter>(
ee: T,
context: Context
context: Context,
ee: T
): T {
const map = this._getPatchMap(ee);
if (map !== undefined) return ee;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,8 @@ for (const contextManagerClass of [
contextManager.bind(context, () => {
assert.strictEqual(contextManager.active(), context);
assert.strictEqual(otherContextManager.active(), otherContext);
}));
})
);
fn();
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ export class ZoneContextManager implements ContextManager {
}

/**
* @param target Function to be executed within the context
* @param context A context (span) to be executed within target function
* @param target Function to be executed within the context
*/
private _bindFunction<T extends Function>(target: T, context: Context): T {
private _bindFunction<T extends Function>(context: Context, target: T): T {
const manager = this;
const contextWrapper = function (this: any, ...args: unknown[]) {
return manager.with(context, () => target.apply(this, args));
Expand All @@ -68,10 +68,10 @@ export class ZoneContextManager implements ContextManager {
}

/**
* @param obj target object on which the listeners will be patched
* @param context A context (span) to be bind to target
* @param obj target object on which the listeners will be patched
*/
private _bindListener<T>(obj: T, context: Context): T {
private _bindListener<T>(context: Context, obj: T): T {
const target = (obj as unknown) as TargetWithEvents;
if (target.__ot_listeners !== undefined) {
return obj;
Expand Down Expand Up @@ -212,9 +212,9 @@ export class ZoneContextManager implements ContextManager {
context = this.active();
}
if (typeof target === 'function') {
return this._bindFunction(target, context);
return this._bindFunction(context, target);
} else if (isListenerObject(target)) {
this._bindListener(target, context);
this._bindListener(context, target);
}
return (target as unknown) as T;
}
Expand Down
8 changes: 4 additions & 4 deletions packages/opentelemetry-web/src/StackContextManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ export class StackContextManager implements ContextManager {

/**
*
* @param target Function to be executed within the context
* @param context
* @param target Function to be executed within the context
*/
private _bindFunction<T extends Function>(
target: T,
context = ROOT_CONTEXT
context = ROOT_CONTEXT,
target: T
): T {
const manager = this;
const contextWrapper = function (this: unknown, ...args: unknown[]) {
Expand Down Expand Up @@ -72,7 +72,7 @@ export class StackContextManager implements ContextManager {
context = this.active();
}
if (typeof target === 'function') {
return this._bindFunction(target, context);
return this._bindFunction(context, target);
}
return target;
}
Expand Down