-
-
Notifications
You must be signed in to change notification settings - Fork 130
[RFC] Singularity #10
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
Conversation
…e APIs in the future Let's face it. There can only be one loop. If we accept this simple fact, we can build much nicer APIs that no longer have to awkwardly pass this loop object around that already is a semi-global context object leaking all over the system.
This is honestly one of the few moments I really like the idea of some global state. Without even going deed into the code it will make writing react applications considerable simpler as we don't have to pass the loop around anymore (there can only be ONE anyway). I'm really in favor of the PR 👍 ! |
If the idea here is that I can do:
instead of
I'm +1000 for this. Passing the event loop around is a real PITA. |
I was aiming for something like:
|
@@ -59,11 +52,11 @@ public function isPeriodic() | |||
|
|||
public function isActive() | |||
{ | |||
return $this->loop->isTimerActive($this); | |||
return l\loop()->isTimerActive($this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO it makes sense to no longer expose the loop as part of the timer interface (i.e. drop the getLoop()
method).
Though arguably, I'm also pro exposing the one loop via global state.
However, I consider this very change a design flaw. Unless we move this logic to the timer itself, it actually does have a dependency on the loop. Accessing global state here means we're using a leaky abstraction and hiding the dependencies this particular class has. Also, this does make testing harder (no built-in mechanism to mock the loop).
I agree, this appears to one of the few places where it actually makes sense to add global state. If only for convenience (and hence a cleaner design for components building on top of this) 👍 I've toyed with the concept of combing several loops for some time already, but even then there's only ever a single "top" loop. So while I agree it makes sense to add a global accessor (let's face it, we're talking about a beloved singleton), I tend to disagree with actually using it in any of our methods (except for constructors). |
👍 - Would also like to see the loop start automatically. |
|
||
class Factory | ||
{ | ||
public static function create() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this stay and just return loop();
?
This PR has been here for well over a year. Anyone who would like to comment has had the opportunity. Can we get a yes or no on this? |
Is there any case when having two loops is helpful? |
This pull request serves as a place for discussing some radical changes to the react API that I've been playing with. This is just a concept. It's just an idea. It needs a lot more work before the rest of react could use it.
Feel free to share your thoughts here though.