Skip to content

Commit

Permalink
fix(subscribe): allow interop with Monio and other libraries that pat…
Browse files Browse the repository at this point in the history
…ch function bind

* fix(subscribe): allows functions where bind has been patched to be weird

Apparently, code exists in the wild that will patch function bind to do something other than return a function that will execute the function instance, so we cannot rely on bind.

Resolves #6783

* refactor: Alternative bind approach

* chore: update side-effects golden files.

* chore: Add comment about why bind is captured

* chore: update side-effect files again
  • Loading branch information
benlesh authored Feb 8, 2022
1 parent e43063a commit 0ab91eb
Show file tree
Hide file tree
Showing 14 changed files with 64 additions and 5 deletions.
2 changes: 1 addition & 1 deletion integration/side-effects/snapshots/esm/ajax.js
Original file line number Diff line number Diff line change
@@ -1 +1 @@

const _bind = Function.prototype.bind;
2 changes: 2 additions & 0 deletions integration/side-effects/snapshots/esm/fetch.js
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
import "tslib";

const _bind = Function.prototype.bind;
2 changes: 2 additions & 0 deletions integration/side-effects/snapshots/esm/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import "tslib";

const _bind = Function.prototype.bind;

var NotificationKind;

(function(NotificationKind) {
Expand Down
2 changes: 2 additions & 0 deletions integration/side-effects/snapshots/esm/operators.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import "tslib";

const _bind = Function.prototype.bind;

var NotificationKind;

(function(NotificationKind) {
Expand Down
2 changes: 2 additions & 0 deletions integration/side-effects/snapshots/esm/testing.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import "tslib";

const _bind = Function.prototype.bind;

var NotificationKind;

(function(NotificationKind) {
Expand Down
2 changes: 1 addition & 1 deletion integration/side-effects/snapshots/esm/websocket.js
Original file line number Diff line number Diff line change
@@ -1 +1 @@

const _bind = Function.prototype.bind;
2 changes: 2 additions & 0 deletions integration/side-effects/snapshots/esm5/ajax.js
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
import "tslib";

var _bind = Function.prototype.bind;
2 changes: 2 additions & 0 deletions integration/side-effects/snapshots/esm5/fetch.js
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
import "tslib";

var _bind = Function.prototype.bind;
2 changes: 2 additions & 0 deletions integration/side-effects/snapshots/esm5/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import "tslib";

var _bind = Function.prototype.bind;

var NotificationKind;

(function(NotificationKind) {
Expand Down
2 changes: 2 additions & 0 deletions integration/side-effects/snapshots/esm5/operators.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import "tslib";

var _bind = Function.prototype.bind;

var NotificationKind;

(function(NotificationKind) {
Expand Down
2 changes: 2 additions & 0 deletions integration/side-effects/snapshots/esm5/testing.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import "tslib";

var _bind = Function.prototype.bind;

var NotificationKind;

(function(NotificationKind) {
Expand Down
2 changes: 2 additions & 0 deletions integration/side-effects/snapshots/esm5/websocket.js
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
import "tslib";

var _bind = Function.prototype.bind;
28 changes: 28 additions & 0 deletions spec/Observable-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,34 @@ describe('Observable', () => {
});

describe('subscribe', () => {
it('should work with handlers with hacked bind methods', () => {
const source = of('Hi');
const results: any[] = [];
const next = function (value: string) {
results.push(value);
}
next.bind = () => { /* lol */};

const complete = function () {
results.push('done');
}
complete.bind = () => { /* lol */};

source.subscribe({ next, complete });
expect(results).to.deep.equal(['Hi', 'done']);
});

it('should work with handlers with hacked bind methods, in the error case', () => {
const source = throwError(() => 'an error');
const results: any[] = [];
const error = function (value: string) {
results.push(value);
}

source.subscribe({ error });
expect(results).to.deep.equal(['an error']);
});

it('should be synchronous', () => {
let subscribed = false;
let nexted: string;
Expand Down
17 changes: 14 additions & 3 deletions src/internal/Subscriber.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,17 @@ export class Subscriber<T> extends Subscription implements Observer<T> {
}
}

/**
* This bind is captured here because we want to be able to have
* compatibility with monoid libraries that tend to use a method named
* `bind`. In particular, a library called Monio requires this.
*/
const _bind = Function.prototype.bind;

function bind<Fn extends (...args: any[]) => any>(fn: Fn, thisArg: any): Fn {
return _bind.call(fn, thisArg);
}

export class SafeSubscriber<T> extends Subscriber<T> {
constructor(
observerOrNext?: Partial<Observer<T>> | ((value: T) => void) | null,
Expand Down Expand Up @@ -166,9 +177,9 @@ export class SafeSubscriber<T> extends Subscriber<T> {
} else {
context = observerOrNext;
}
next = next?.bind(context);
error = error?.bind(context);
complete = complete?.bind(context);
next = next && bind(next, context);
error = error && bind(error, context);
complete = complete && bind(complete, context);
}

// Once we set the destination, the superclass `Subscriber` will
Expand Down

0 comments on commit 0ab91eb

Please sign in to comment.