Skip to content

Conversation

@dustincleveland
Copy link

This pull request should fix issue #3, removing the error that is thrown when an event has no subscribers. During this fix, I also noticed that several tests were written using the bind() function, like so:

expect(pubService.$sub.bind(undefined)).toThrow();

This syntax binds the this keyword of the function to undefined. I believe the intent here was to pass undefined as the first argument to the function. To fix this issue, I replaced the test with arrow functions:

expect(() => pubService.$sub(undefined)).toThrow();

This syntax should correctly pass undefined as the event argument of the $sub method.

Dustin Cleveland added 3 commits March 25, 2017 10:58
…re passed as method arguments instead of setting 'this'.
…lt value being emitted when first subscribing to an event.
@dustincleveland
Copy link
Author

When subscribing to an undefined event, new events are added as BehaviorSubjects (see line 24 of angular2-pubsub.service.ts: this.events[event] = new BehaviorSubject<any>(0);). This immediately emits the default value of '0' to the subscriber. This can cause subtle bugs in consumer code where this initial event is not expected to occur. Replacing BehaviorSubject with ReplaySubject should allow the solution to behave as it currently does, but prevents that initial event.

FYI: I meant for this to be a second pull request, but didn't realize how GitHub's pull request system works... in the future, I will create feature branches so that the pull requests stay independent of one another (I think that's the right way?).

@sqlProvider
Copy link
Owner

Thanks Dustin.

@sqlProvider sqlProvider merged commit 66b3a99 into sqlProvider:master Mar 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants