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.

Sign up for GitHub

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

Jump to bottom

MimicStream should pass its listeners to imitated stream #49

Closed
Hypnosphi opened this issue Jun 9, 2016 · 7 comments
Closed

MimicStream should pass its listeners to imitated stream #49

Hypnosphi opened this issue Jun 9, 2016 · 7 comments
Labels
issue is suggestion weight 2 (could)

Comments

@Hypnosphi
Copy link
Contributor

Hypnosphi commented Jun 9, 2016 •
edited
Loading

Since 3.0.0, imitate works only when placed before listeners (it's an opposite of issue #41 ).

As docs say, imitate changes this current MimicStream to imitate the other given stream. In my opinion, this means that no matter when do we add a listener to MimicStream, in the past or in the future, it should listen to the imitated stream (and stop listening to previously imitated stream, if present).
@Hypnosphi Hypnosphi changed the title MimicStream should pass it's listeners to imitated stream MimicStream should pass its listeners to imitated stream Jun 9, 2016
@staltz
Copy link
Owner

staltz commented Jun 9, 2016 •
edited
Loading

You're totally correct, we should do this.

Note for self: we could implement this by adding early listeners to a queue if there is no target on the MimicStream. Then, when imitate, consume the queue.

@Hypnosphi Hypnosphi mentioned this issue Jun 9, 2016
Closed
staltz added a commit that referenced this issue Jun 12, 2016
@staltz
fix(imitate): fix cyclic execution leak, and refactor
8a432b6 
Rewrite MimicStream with a different approach. Adds a hidden listener field (_hil) to Stream, which
is a special listener which does not trigger start nor stop when added/removed. imitate just adds
the MimicStream as a hidden listener on the target Stream, and MimicStream has its own listeners, it
does not redirect them to the target. This new approach breaks the usage of imitate for
Directed-Acyclic-Graphs, but we anyway intend MimicStream to be used _only_ to implement a cycle in
the stream graph. So this commit removes some tests.

Fixes bug #51, deprecates/invalidates issue #49.
@staltz
Copy link
Owner

staltz commented Jun 12, 2016

Ok, actually I think there is no valid use for calling imitate after addListener, so this issue becomes deprecated. Read 8a432b6 for more details.

If we find a very valid use case for imitate after addListener, we can reopen this issue.

@staltz staltz closed this as completed Jun 12, 2016
@Hypnosphi
Copy link
Contributor Author

Can we possibly reopen this now to support dynamic graph construction?

@staltz
Copy link
Owner

staltz commented Jun 14, 2016

What's the use case?

@Hypnosphi Hypnosphi mentioned this issue Jun 15, 2016
Closed
@staltz
Copy link
Owner

staltz commented Jun 16, 2016

http://www.webpackbin.com/N12gpWq4W

@staltz staltz reopened this Jun 16, 2016
@TylorS
Copy link
Collaborator

TylorS commented Sep 25, 2016 •
edited
Loading

I believe this is covered now already? Also MimicStream has been removed

@staltz
Copy link
Owner

staltz commented Sep 25, 2016

Yeah

@staltz staltz closed this as completed Sep 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
issue is suggestion weight 2 (could)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@staltz @TylorS @Hypnosphi