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

Support the Channel as listener #12

Merged
merged 9 commits into from
May 6, 2020

Conversation

procot
Copy link

@procot procot commented Sep 25, 2019

No description provided.

@procot
Copy link
Author

procot commented Sep 25, 2019

@vitalets И кстати что там с документацией? Надо что-то поправлять, чтобы здесь https://vitalets.github.io/chnl/ появился мой добавленный метод?

src/channel.js Outdated
* @param {Channel} channel
*/
addProxyChannel(channel) {
this._proxyChannels.push(channel);
Copy link
Collaborator

Choose a reason for hiding this comment

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

channel instanceof Channel?
And also this._proxyChannels.includes(channel)

src/channel.js Outdated
* Register the channel to which events should be proxied
* @param {Channel} channel
*/
addProxyChannel(channel) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I also propose to add removeProxyChannel/removeAllProxyChannels?

@vitalets
Copy link
Owner

vitalets commented Oct 4, 2019

Please, use English )

src/channel.js Outdated
@@ -1,8 +1,12 @@
const innerEvents = [
'onListenerAdded',
'onListenerRemoved',
'onProxyChannelAdded',
Copy link
Owner

Choose a reason for hiding this comment

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

All of these is not needed:
'onProxyChannelAdded',
'onProxyChannelRemoved',
'onFirstProxyChannelAdded',
'onLastProxyChannelRemoved'

Proxying is just adding special listener.
Please update pr.

src/channel.js Outdated
@@ -24,6 +28,7 @@ const innerEvents = [
export default class Channel {
constructor(name, noInnerEvents) {
this._listeners = [];
this._proxyChannels = [];
Copy link
Owner

Choose a reason for hiding this comment

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

Not needed.

@procot procot requested a review from vitalets October 8, 2019 14:55
proxyChannel.addListener(spy);
channel.proxyTo(proxyChannel);
t.is(spy.callCount, 0);
channel.dispatch();
Copy link
Owner

Choose a reason for hiding this comment

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

Let's pass some data to ensure that data also proxied.

src/channel.js Outdated
*/
_ensureChannel(channel) {
if (!(channel instanceof Channel)) {
throw new Error('Channel ' + this._name + ': proxyChannel doesn\'t instance of Channel');
Copy link
Owner

Choose a reason for hiding this comment

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

`Channel ${this._name}: proxyChannel doesn't instance of Channel`

Copy link
Collaborator

Choose a reason for hiding this comment

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

doesn't -> is not

@procot procot changed the title Проксируем события в другие каналы Support Channel as listeners Apr 30, 2020
@procot procot changed the title Support Channel as listeners Support the Channel as listener Apr 30, 2020
@procot
Copy link
Author

procot commented Apr 30, 2020

@vitalets @stamoern
Please check this PR.
I added support a channel as listener. Now we can using:

const channel1 = new Channel();
const channel2 = new Channel();
const channel3 = new Channel();

channel1.addListener(channel2);
channel1.addOnceListener(channel3);
channel1.hasListener(channel2);

new Channel.ReactSubscription(component, [
  { channel: channel1, listener: channel2 }
])

@procot procot requested a review from vitalets April 30, 2020 18:11
@@ -55,8 +57,8 @@ export default class SubscriptionItem {
if (event && typeof event !== 'string') {
throw new Error('Event should be string');
}
if (!listener || typeof listener !== 'function') {
throw new Error('Listener should be function');
if (!listener || (typeof listener !== 'function' && !(listener instanceof Channel))) {
Copy link
Owner

Choose a reason for hiding this comment

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

lets add static Channel.isValidListener() to avoid repeating typeof listener !== 'function' && !(listener instanceof Channel)

@procot procot requested a review from vitalets May 1, 2020 13:38
Copy link
Owner

@vitalets vitalets left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@vitalets vitalets merged commit ac1742e into vitalets:master May 6, 2020
@procot procot deleted the proxyToAnotherChannel branch May 6, 2020 16:50
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.

3 participants