wpengine / faustjs

Faust.js™ - The Headless WordPress Framework
https://faustjs.org
Other
1.43k stars 131 forks source link

Add filter for search_pattern in handle_generate_endpoint #1872

Open cguagenti opened 6 months ago

cguagenti commented 6 months ago

Context:

I'm testing Faust with WordPress Bedrock, and due to the different structure, when previewing content, it results in a 404 error.

This issue is caused by the $search_pattern in handle_generate_endpoint(). https://github.com/wpengine/faustjs/blob/002687fafd0589d718c3d4fa3403b9ff2a3bb88c/plugins/faustwp/includes/auth/callbacks.php#L24-L29

Because Bedrock returns the prefix /wp/ in site_url( '/generate', 'relative' ) (/wp/generate) while $_SERVER['REQUEST_URI'] contains only /generate[...], it always fails at the statement below:

if ( ! preg_match( $search_pattern, $_SERVER['REQUEST_URI'] ) ) { // phpcs:ignore WordPress.Security
    return;
}

Suggestion

Since site_url() returns the URL for the current site where WordPress applications are accessible, the best way to move forward is to use a callback filter (apply_filters) so anyone can customize it with a callback filter.

/**
* Filter 'faustwp_generate_endpoint_search_pattern'.
*
* @param string  $value   The search pattern.
*/
$search_pattern = apply_filters( 'faustwp_generate_endpoint_search_pattern', $search_pattern );

Example of how to use the filter

add_filter('faustwp_generate_endpoint_search_pattern', function ($value) {
    return str_replace('/wp/', '/', $value);
});
jan-clockworkwp commented 5 months ago

Hey @theodesp, I am confirming that @cguagenti suggestion solves previews pages 404 issue on non-WordPress standard installations where admin url differs from the siteurl. Can we hope to have that filter added to the official package, or maybe as an option in the plugin options? Many thanks.

ChrisWiegman commented 4 months ago

I like your idea here but I don't think it solves the whole problem. We actually have a 2nd ticket right now, #1882, that looks to solve Bedrock paths with a JS filter in Faust. In isolation both approaches would solve the specific symptoms but the larger issue of Bedrock and similar architecture remains. I would rather solve this upstream where 1. a single change will make sure Bedrock and similar work without multiple filters, etc and 2. it should be solvable whether you're using Faust or not. To further discuss an approach I've opened https://github.com/wp-graphql/wp-graphql/issues/3145 as a result.

justlevine commented 4 months ago

On quick review, the underlying issue is that Faust is conflating/inconsistently using the site_url, home_url, and internal Faust frontend_uri, both on the backend and frontend sides.

(I assume the decision to create this endpoint on parse_request instead of as a real endpoint (RESTful or otherwise) was borne out of this as well ).

The immediate solution from a code quality, tech debt and DX pov would be to drop the optimistic home_url === site_url === wpUrl, and use the correct ones on the frontend (long term also in the plugin, but that's beyond the scope if this particular SERVER['REQUEST_URI] check).

I'm assuming that's the premise for the exploration happening in https://github.com/wp-graphql/wp-graphql/issues/3145, but I'm not seeing the why we would want to query the siteUrl via GraphQL before we use it elsewhere in the frontend (the request in that ticket).

This would seem to make much more sense as an optional .env constant (that inherits the home url if unset)

@ChrisWiegman can you confirm/correct my assessment?

jan-clockworkwp commented 2 months ago

@ChrisWiegman @theodesp was there any progress done on handling different than default WordPress wp-admin setups? I can see @theodesp has done PR that could help with FE issues, but it is in review stage.

Currently, we need to apply various tweaks to the front-end and back-end, but these tweaks cannot solve everything we would like to address. As you can imagine, making tweaks to the plugin and Faust.js packages is not ideal for projects in staging or production phases.

Thank you for your input.

ChrisWiegman commented 2 months ago

I’m no longer with WP Engine. Removing myself from this ticket.