-
-
Notifications
You must be signed in to change notification settings - Fork 147
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
Replace the queue() function with a static Queue class #82
Conversation
This makes the Queue class autoloadable and fixes cases were a consumer wants to set a queue driver via a bootstrap file loaded from the autoload/files section of the composer.json. The bootstrap file might be loaded before the functions.php and the queue() function is then not available.
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.
This whole feature is currently undocumented, so I'm not sure how either the existing version or the updated version is preferable from a consumer perspective?
What do you think? Should we document this in our README or is this merely internal?
I'm not sure what you mean here. The only difference is the API, behavior is unchanged. I've update the PR description, probably its clearer now what changed.
Good point, at the moment it is internal. The overall behavior is unchanged to v2. The original motivation for adding a queue has been to reduce recursion and avoid blowing up the call stack (#22). A side effect is that you can swap out the queue implementation to make react/promise guarantee future-turn resolution (#4), eg. required by the promises/a+ spec. But this is currently undocumented and not tested. |
Ping @clue |
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.
Apparently, this is an unreleased feature that is both undocumented and untested, so… ¯\_(ツ)_/¯
@clue Just to be clear, this feature is tested.
This is untested, not documented and currently considered an internal feature. I'm going to make this clearer by marking everything related |
We allow swapping out the queue implementation and this must be done in a bootstrap file before executing anything from react/promise.
Consider the case, were a user defines a bootstrap file in the composer.json were the queue implementation is changed.
The
bootstrap.php
contains the code for swapping out the queue implementation.Depending on the order in
vendor/composer/autoload_files.php
, ourfunctions.php
might be loaded after thebootstrap.php
thus making thequeue()
function unavailable to the bootstrap file.This patch replaces the
queue()
function by a static class which is autoloadable.The only difference is in the API, the behavior is unchanged (note, that this feature is not released yet).
Enqueuing a task:
Swapping out the queue implementation: