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

ServerSideRender does not handle usesContext #34882

Open
sc0ttkclark opened this issue Sep 16, 2021 · 8 comments
Open

ServerSideRender does not handle usesContext #34882

sc0ttkclark opened this issue Sep 16, 2021 · 8 comments
Labels
Needs Dev Ready for, and needs developer efforts [Package] Server Side Render /packages/server-side-render [Type] Bug An existing feature does not function as intended

Comments

@sc0ttkclark
Copy link

sc0ttkclark commented Sep 16, 2021

What problem does this address?

I have a block that supports usesContext and it works if you're dealing with client-side rendering.

For server side rendering via ServerSideRender -- that's where things drop off. When rendering the block on the frontend (output) it will receive context fine. However, the context is never passed into the REST API endpoint to receive the preview.

I have to manually build and pass this in myself.

Here's an example of what the workaround right now is (simplified for example purposes).

let urlQueryArgs = {};

usesContext.forEach( contextName => {
	urlQueryArgs.myContext[ contextName ] = context[ contextName ] ?? null;
} );

return (
	<ServerSideRender
		block={ blockName }
		attributes={ attributes }
		urlQueryArgs={ urlQueryArgs }
	/>
);

What is your proposed solution?

I think a good solution would be to add another attributes-like optional argument context={ context } to ServerSideRender, to pass in the context if you received it / use it from your edit component.

This would also need to include additional code in the REST API endpoint WP_REST_Block_Renderer_Controller so that it can support the new passed context information. Currently, it uses a context param in the endpoint to denote view vs edit. So this would need to be a new param name like contextValues and accept an array, similar to how the attributes param works.

All of this would give block authors more flexibility with their server side rendered blocks than they have right now.

An example implementation of the ServerSideRender usage would be like this:

return (
	<ServerSideRender
		block={ blockName }
		attributes={ attributes }
		context={ context }
	/>
);
@Mamaduka Mamaduka added [Package] Server Side Render /packages/server-side-render [Type] Enhancement A suggestion for improvement. labels Sep 16, 2021
@sc0ttkclark sc0ttkclark changed the title ServerSideRender does not handle Context ServerSideRender does not handle usesContext Sep 16, 2021
@ntsekouras
Copy link
Contributor

👋 @sc0ttkclark !

I'm not sure I understand your use case and the challenges you face. Can you expand a bit more?

In general here you can find some information of how Block context can be used.

Did you mean how to access the code in the php side? If it is you can access the context through $block parameter. An example is here.

Happy to help if you have any questions 😄

@sc0ttkclark
Copy link
Author

Hey @ntsekouras, what's up!

Thanks for getting back to me about this.

For clarification, I have a block that usesContext: [ "postId", "postType", "query" ] and I don't receive any of those values from the REST API WP_REST_Block_Renderer_Controller endpoint. The attributes are the only thing that passes into ServerSideRender for the REST API request from the block. The post_id gets passed into the REST API request from the global context and does not represent the current postId value.

This gets tricky when you use the Query Loop block, which can manipulate postId and postType for it's purposes (yay!) but that can't be used from the REST API preview context :(

@sc0ttkclark
Copy link
Author

One more point to make sure it was clear -- if usesContext provides the correct context when doing a server side render -- then we ought to figure out how to pass context into the block preview REST API.

Using the $block->context is great on server side render, everything is there except on block previews via REST API.

@gziolo
Copy link
Member

gziolo commented Sep 17, 2021

I can see how it could get missed when the block context was introduced. I think the solution proposed is worth exploring.

Looking at https://developer.wordpress.org/rest-api/reference/rendered-blocks/, I see that context is already used so it has to be a different name. Otherwise, it should work.

@gziolo gziolo added [Type] Bug An existing feature does not function as intended Needs Dev Ready for, and needs developer efforts and removed [Type] Enhancement A suggestion for improvement. labels Sep 17, 2021
@Mamaduka
Copy link
Member

What do you think about blockContext as a new name?

@gziolo
Copy link
Member

gziolo commented Sep 17, 2021

What do you think about blockContext as a new name?

In the REST API endpoint for block types, it's uses_context: https://developer.wordpress.org/rest-api/reference/block-types/. It reminds me that we had to rename that field in PHP from context to avoid name clashing in REST API 😅

In this case, it isn't really uses_context because we have to pass computed values read from context, so maybe context_values? We can sort out the name later when we see how it needs to be handled in PHP. We will have to inject this context somewhere here:

https://github.com/WordPress/wordpress-develop/blob/ab9b51e5e31a565c6da5398d4af51d4749168a90/src/wp-includes/rest-api/endpoints/class-wp-rest-block-renderer-controller.php#L178-L186

I'm not sure how much work it would require to override get_item from this REST API controller in Gutenberg. Maybe we will have to split into two parts - a core patch for REST API and a PR for passing the context values.

@Mamaduka
Copy link
Member

Maybe we will have to split into two parts - a core patch for REST API and a PR for passing the context values.

I like this approach more. The overriding controller would mean supporting it until the plugin requires WP 5.9.

I'll try to create both PRs next week if there are no other takers.

@sc0ttkclark
Copy link
Author

sc0ttkclark commented Sep 17, 2021

All this sounds great to me, for now I'm using my own custom URL argument that I look for in the REST API, in the future I will switch that to the new param name (and in a future WP version I can remove that custom handling).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Dev Ready for, and needs developer efforts [Package] Server Side Render /packages/server-side-render [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

4 participants