-
Notifications
You must be signed in to change notification settings - Fork 466
feat: introduce GraphQL event registry system and event registration hook #3376
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
base: develop
Are you sure you want to change the base?
feat: introduce GraphQL event registry system and event registration hook #3376
Conversation
| * | ||
| * @return void | ||
| */ | ||
| public static function register_event( $name, $hook_name, $callback, $priority = 10, $arg_count = 1 ) { |
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.
Method register_event has 5 arguments (exceeds 4 allowed). Consider refactoring.
|
Code Climate has analyzed commit b14c4fd and detected 1 issue on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
| * | ||
| * @return void | ||
| */ | ||
| public static function register_event( $name, $hook_name, $callback, $priority = 10, $arg_count = 1 ) { |
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.
Method register_event has 5 arguments (exceeds 4 allowed). Consider refactoring.
|
Hey @justlevine and @jasonbahl, just wanted to follow up on this PR and check whether it’s being actively considered, especially regarding the proposed public functions like The reason I ask is that we’re currently experimenting with a headless webhooks plugin built around this PR (POC stage for now) and would love to hear your thoughts or suggestions on how we might help move this PR forward. Appreciate any feedback or direction you can share! |
Hey @theodesp I actually wanted to ask you about this on last night's community call, since I wasn't sure what the scope of this PR is/should be. As of now this PR adds an event store, but doesn't actually use it for anything. Personally when there's no scope or RFP, I prefer we minimally dogfood our APIs in core, otherwise they risk turning into tech debt immediately. Once this scope is clarified, next step would be wpunit tests I assume. |
|
Hey David I gueess I agree that without an immediate consumer in core, there’s a risk of adding speculative APIs. We’re happy to port this functionality into our headless webhooks plugin for now, as it doesn’t have any deep dependencies on WPGraphQL itself beyond hooking into the WPGraphQL class actions. This way we can continue experimenting, get real-world usage behind it without the pressure of it needing to live in core immediately. If and when a clearer scope or use case emerges for core, we’d be happy to revisit it or upstream improvements based on what we learn. In the meantime, we'll keep it cleanly contained in our plugin. Thanks for the heads-up, and really appreciate the context. |
|
IIRC usage candidates discussed previously for this API were:
|
|
As I think has been pointed out, the idea was to move the event system out of Smart Cache and into core so that other things, like webhooks and logging, could take advantage of it. Do we need to move those parts of the smart cache as part of this PR too? |
|
@theodesp I have shoulder surgery scheduled for June 5 and likely will not get to approving this or super deeply discussing further until after that. In short, yes, it is seriously being considered, but unfortunately I'm going to be a bottleneck for a bit longer and delay the momentum. Hope to get momentum picked up as I begin my recovery from surgery. |
|
Ok, so, some initial thoughts: WordPress already has a generic hook/filter system, so we need this to go beyond that and be more unique to WPGraphQL. At the heart of WPGraphQL is the concept of Nodes and I believe our "event emitter" should center around nodes. If we take a look at WPGraphQL Smart Cache or it's spiritual predecessor – the WPGatsby Action Monitor, both systems are centered around determining when CRUD events have happened to nodes. I believe we need to centralize around this concept. As a WPGraphQL user, I want to be notified when a Node has been created, updated or deleted. This allows me to:
In both ActionMonitor and WPGraphQL Smart Cache, we have a lot of logic that does things like checks the state of a post to see if it's a public post or not, or check the state of a user to see if it's public or not (i.e. does it have published posts or not) I think it would be good to centralize some of these concepts into whatever API we come up with, so that when we query for Nodes, we can also get updates about Nodes. If we're too broad with what a "graphql event" is, then it's not much different than a regular I think right now, the state of this PR is just introducing another Possibly one thing that would help would be to come up with some real-world use cases to solve (i.e. how could we accomplish the same thing that WPGraphQL Smart Cache does in a more centralized event-emitter API like this) |
Here's prior art of WPGatsby and the even older Jamstack Deployments integrations for WPGraphQL for GF: |
|
Ok, so after thinking a bit more about this, I think what we ultimately want to start working on here is the underlying api for GraphQL Subscriptions. GraphQL Subscriptions are a 3rd root type (alongside Query and Mutation), and the intent is to allow for real-time (or near-real-time) updates from a GraphQL Server. With this in mind, we should be able to start building the underlying event-tracking and emitting system, with the ultimate goal of exposing subscriptions in the schema when we feel like we have valid options for the subscriptions transport, but we can still enable the event tracking and emitting even before we expose Subscriptions to the schema, to solve for existing use cases such as cache invalidation (WPGraphQL Smart Cache). I'll spend some more time working up a more formal RFC, but below is my initial thought on what an API like this could look like that would allow us to register "event listeners" in a central wpgraphql-esque way, with the goal of solving current situations like cache invalidation and the ultimate goal of bringing proper GraphQL Subscriptions to WPGraphQL. register_graphql_subscriptionHere's what I'm thinking for a // hook in to plugins_loaded or later to make sure WPGraphQL is loaded
// and `register_graphql_subscription` exists.
// we don't hook into a graphql specific hook like `graphql_register_types`
// because we will be tracking events outside of graphql and want
// as wide of a scope as possible to track a wide variety of actions in
// WordPress and we shouldn't require the GraphQL schema, etc to bootstrap.
add_action( 'plugins_loaded', function() {
// register a subscription. This not only will add a field to the root Subscription type,
// but will also track events in WordPress and emit messages that will trigger subscription operations.
register_graphql_subscription(
// $name: The name of the registered subscription
'postUpdated',
// $config: The config array for the subscription to be registered
[
'description' => __( 'Describe the subscription. This will map to the description in the schema', 'wp-graphql' ),
// This is an array of events to "subscribe" to in WordPress.
// Each event in this array will be mounted as `add_action( $event_name, $callback, $args, $priority )`
'subscribe' => [
'wp_insert_post' => function( $dispatcher, $event_args ) {
// early return if the event doesn't qualify
if ( 'xxx' !== $event_args['yyy'] ) {
return;
}
// Build the payload.
// Potentially we use a class here instead of an arbitrary array to ensure some consistency in payloads
// i.e. `$payload = new \WPGraphQL\SubscriptionPayload( ... );` or similar
$payload = [
'nodeId' => deriveNodeIdFromEventArgs( $event_args ),
'nodeType' => deriveTypeFromEventArgs( $event_args ),
'whateverElseIsRelevant' => deriveOtherRelevantInfoFromEventArgs( $event_args ),
];
// We dispatch an event that will get added to a queue, and ideally processed async
// The queue should be de-duped then processed, by sending events out to whatever
// transport mechanisms are configred.
// The transport mechanisms would be configured separately.
// i.e. websocket, webhook, server-sent-events
// for webhook and SSE subscriptions, WordPress probably will
// need to store the subscription data (custom post type + custom taxonomy)
// so that the queue can be processed by WordPress.
// for websocket subscriptions, the websocket server would need to
// persist the subscription data and handle connections/disconnections, etc.
$dispatcher->dispatch( 'postUpdated', $payload );
},
]
// The fields that will be queryable on the subscription
'fields' => [
'post' => [
'type' => 'Post',
'description' => __( 'The post that was updated', 'wp-graphql' ),
],
],
// The args that allow a user to refine/filter the subscription
'args' => [
// allow the subscription to be filtered by status
'status' => [
'type' => 'PostStatusEnum',
],
],
// The resolver to execute when the subscription operation is executed.
// The $payload is the root object that will be set by the payload that was emitted
// The $args will be the args defined by the user in the document
'resolve' => function( $payload, $args, $context, $info ) {
// The $payload would be the context that was sent out with the dispatch
$post = derivePostFromPayload( $payload );
if ( ! $post ) {
return null;
}
// Bail if the args on the subscriptions don't match the node we subscribed to
if ( isset( $args['status'] ) && $args['status'] !== $post->post_status ) {
return;
}
// If the event is relevant still, we resolve the subscription
return new \WPGraphQL\Model\Post( $post_id );
}
]
);
} |
What does this implement/fix? Explain your changes.
This PR introduces a new event registry system for WPGraphQL to centralize the registration and handling of WordPress hooks as GraphQL events. It includes:
EventRegistryclass to manage event configurations and attach them to WordPress actions.register_graphql_event()helper function for external plugins or themes to declaratively register events.EventMonitorutility to track dispatched events via hooks likewpgraphql_event_tracked_{event}.Does this close any currently open issues?
Draft PR for #2517
Any other comments?
Looking forward to feedback on the pattern and implementation approach and other suggestions around testing and documentation
Example Usage of
register_graphql_eventTry to save a post and you'll see the response containing the
['post_id' => $post_id]You can also inspect the error.log