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

sandbox.collect() #322

Open
atesgoral opened this issue Oct 30, 2013 · 6 comments
Open

sandbox.collect() #322

atesgoral opened this issue Oct 30, 2013 · 6 comments

Comments

@atesgoral
Copy link
Member

sandbox.emit() is good when you can "fire and forget" an event. But there are times when a component needs to collect results from other components (for when a Service Locator implementation is needed). This can still be achieved with sandbox.emit() through some hacky ways like:

  1. Passing in a bucket (empty Array) as an event argument that can be populated by listeners.
  2. Passing in a callback as an event argument that can be called by listeners.

However, this only works when all listeners can return results synchronously. If results are resolved asynchronously, there is still another hack that can be applied: Let listeners return a Promise right away, and then resolve the underlying Deferred asynchronously. But this solution would involve some ugly boilerplate code on the application side every time such asynchronous query needs to be executed. Guessing something like:

var resultsBucket = [];

this.sandbox.emit("checkState", resultsBucket);

$.when.apply($, resultsBucket).done(function () {
    // Oh my goodness...
    var states = Array.prototype.reduce.call(arguments, function (states, state) {
        return states.concat(state);
    }, []); // I don't even know if I got this right

    // states should now be an array of asynchronously collected component states
});

Of course, an application can write a utility to hide this ugliness, but I was thinking that this sort of asynch collection facility could be built into Aura. Maybe something like:

this.sandbox.collect("checkState").done(function (states) {
    // states should now be an array of asynchronously collected component states
});

I've already proactively issued a pull request for EventEmitter2 to also have a .collect() method there -- have no idea if it will fly: EventEmitter2/EventEmitter2#102

At the worst case, this can be achieved entirely within Aura by a hack (or shall we say, cleverness) as the above.

@Pickachu
Copy link
Contributor

Pickachu commented Nov 2, 2013

👍

@addyosmani
Copy link
Member

I'm really glad to see that you considered upstreaming this behaviour to EE2 as its useful in the general event space.

For Aura, would you be interested in putting together a PR that brought this behavior to the project? We could of course punt on this if it does end up being a part of eventemitter2.

My only reservations are probably the benchmarks mentioned in the other thread, which would be great to further optimize on.

@atesgoral
Copy link
Member Author

Will give it a shot (doing it within Aura) soon!

@atesgoral
Copy link
Member Author

I tried adding a mediator.collect() method in Aura, but then the unit tests started failing left and right. The primary reason is that Mediator is directly tapping into EventEmitter's API and the unit tests are also assuming EventEmitter internals.

I think it would have been nicer if Aura would approach the underlying emitter implementation as a black box. I simply got stuck on getting any unit tests running. We could take this as a future improvement (#323) if you all agree.

I'm therefore considering again (EventEmitter2/EventEmitter2#102) landing a .collect() method in EventEmitter (and then in Aura's Sandbox) to leave the rest of Aura alone. Doing it purely in Aura turned out to be messier than I expected.

@atesgoral
Copy link
Member Author

The description for hij1nx/EventEmitter2 has received a "[Not actively developed]" designation. I could potentially take the route of adding .collect() on a fork of EE2 and Aura could use it. It would be an addition to the EE2 interface and not affect performance of .emit().

@webuniverseio
Copy link

Hi guys, I'm a huge fan of scalable javascript architecture and here are my 2 cents - stand alone sandbox module with lots of cool features, would you mind to give me some feed back about it, if you have time? Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants