wordpress-playground
wordpress-playground copied to clipboard
WordPress
issue #1186 add sitemap.*.xml$ to the things that generate a .php request.
What is this PR doing?
#1186 The request is handled as though it is for a static file in packages/php-wasm/universal/src/lib /php-request-handler.ts.
What problem is it solving?
The leading sitemap plugins generate the sitemap.xml file on request
How is the problem addressed?
add sitemap.*.xml$ to the things that generate a .php request.
Testing Instructions
install the plugin https://github.com/RavanH/xml-sitemap-feed for instance and surf to /wp-sitemap.xml for instance.
@rhildred this change looks specific to your plugin and I don't think that it would work well for Playground.
Are there other ways your plugin could detect the request and generate the sitemap? For example it could regenerate it using cron jobs and have it as a static file. How are other plugins handling this?
Alternatively you could add some rewrite rules to WordPress or even to your instance of Playground to redirect the request to a PHP script.
@bgrgicak I wonder why that works with a "regular" WordPress and not with Playground. Could it be that Playground only resolves the URLs ending with / and .php as PHP files? I think Apache takes the opposite approach and resolves static files if they exist, and otherwise it falls back to the main index.php file via the rewrite rules. Could we get rid of the seemsLikeAPHPFile function entirely and follow that same pattern?
I agree @adamziel . I also think that apache looks to see if there is a static file, and then serves from further back in the path if it doesn't exist. Checking for a static file, rather than changing the pattern of seemsLikeAPHPFile is probably the correct approach.
I took some time to explore this today and it's not that simple.
Here is a "working" draft , but it needs more work.
The current logic is, if it's a static file, return file, otherwise run the path as PHP, but that means that every missing static file request will trigger a PHP request.
This wouldn't be that bad in a normal environment, but Playground lazy loads some WordPress assets, and in this implementation before loading that asset a PHP request runs. For example, if you open /wp-admin/ this will attempt to lazy load 68 files which means 68 additional PHP requests. I will find a way to exclude these lazy loaded paths from PHP requests and that should fix the issue.
Aha, the big problem here is Apache has immediate access to the filesystem and knows whether a static file exists or not. Playground doesn't. A chunk of static files is served on https://playground.wordpress.net/ and needs to be requested via HTTP before we know if they're a valid rewrite target.
I'd say a solution here would be to ship a list of those hosted static files with a minified WordPress release, and then Playground could be certain whether a static file exists or not.
One up on that would be assuming that paths ending with .js, .css, .png, .jpg, and .gif are always static – just to avoid risking dispatching a burst of static assets request to PHP only to get 404s in return and having an unresponsive playground for a few seconds.
That being said, a "proper" solution might need to wait considering deeper problems like inability to update site settings.
@rhildred does the sitemap.xml file name matter for your use-case? Or would using sitemap.php unblock you for now?
I am looking at the sitemap.php approach right now. I really just need an array of all of the published routes that I can loop over rather than use "Select.... from wp-posts where post-status = published and post-type in ('page', 'post')
I'd say a solution here would be to ship a list of those hosted static files with a minified WordPress release, and then Playground could be certain whether a static file exists or not.
@adamziel I planned to exclude static files in /wp- folders from being executed like PHP files. This would resolve the issue without a list.
@adamziel I planned to exclude static files in /wp- folders from being executed like PHP files. This would resolve the issue without a list.
@bgrgicak How do you know which files are static, though? /wp-content/plugins/thumbnails/image-360x360.png could be handled by a PHP file just like /sitemap.xml.
@bgrgicak How do you know which files are static, though? /wp-content/plugins/thumbnails/image-360x360.png could be handled by a PHP file just like /sitemap.xml.
I would just assume that all non PHP file in these paths are static. This is why the rule should probably be just for wp-admin and includes. It would be a quick change, compared to having a full list, so it might be a good tradeoff.
@bgrgicak Makes sense as a stop-gap solution, as long as it wouldn't start dispatching 404 /wp-content requests to PHP.
@brandonpayton is this one solved with https://github.com/WordPress/wordpress-playground/pull/1539?
@adamziel this is solved with #1539 when pretty permalinks are enabled within WordPress. This is actually the WordPress default if it can be tested successfully during WordPress install. See:
- wp_install_maybe_enable_pretty_permalinks() is called here
- wp_install_maybe_enable_pretty_permalinks() tries a pretty permalink structure and tests with a wp_remote_get()
But the WP default in Playground is currently no pretty permalinks.
Maybe it is because WordPress is installed during bootWordPress() before Playground is able to satisfy subrequests (just a guess that could be wrong). I was hoping to look into it yet today.
I created an issue for the pretty permalink default here: https://github.com/WordPress/wordpress-playground/issues/1644
@rhildred, thank you for this PR. 🙇 It turns out we were able to fix this and a variety of other issues by reworking request routing in #1539, so we can close this. Until #1644 is fixed, it is necessary to proactively enable pretty permalinks for sitemap.xml support to work properly.