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

Suboptimal rewrite handling causes plugin conflicts #1075

Closed
swissspidy opened this issue Nov 16, 2023 · 6 comments · Fixed by #1096
Closed

Suboptimal rewrite handling causes plugin conflicts #1075

swissspidy opened this issue Nov 16, 2023 · 6 comments · Fixed by #1096
Labels
bug Something isn't working
Milestone

Comments

@swissspidy
Copy link
Collaborator

I ran into constant rewrite issues with Polylang and the PWA plugin as reported here, so I had to disable the latter for now.

Turns out the way the PWA plugin adds and potentially flushes rewrite rules on init is flawed. See Chouby's explanation.

@swissspidy swissspidy added the bug Something isn't working label Nov 16, 2023
@westonruter
Copy link
Collaborator

Actually, the flushing is happening at admin_init. And is adding rewrite rules at init too early?

@swissspidy
Copy link
Collaborator Author

As per the support topic, Polylang deletes the rewrite_rules option so that WP generates it upon next page load.

The PWA code in question here:

/**
* Adds rewrite rules to enable pretty permalinks for the service worker script.
*
* @global WP_Rewrite $wp_rewrite WordPress rewrite component.
*/
function pwa_add_rewrite_rules() {
global $wp_rewrite;
$rewrite_rule_regex = '^wp\.serviceworker$';
$rules = $wp_rewrite->wp_rewrite_rules();
if ( ! isset( $rules[ $rewrite_rule_regex ] ) ) {
// Note: This logic will not be required as part of core merge since rewrite rules are flushed upon DB upgrade (as long as the DB version is bumped).
add_action(
'admin_init',
function () {
flush_rewrite_rules( false ); // phpcs:ignore WordPressVIPMinimum.Functions.RestrictedFunctions.flush_rewrite_rules_flush_rewrite_rules -- Not theme code.
}
);
}
add_rewrite_rule( $rewrite_rule_regex, 'index.php?' . WP_Service_Workers::QUERY_VAR . '=' . WP_Service_Workers::SCOPE_FRONT, 'top' );
add_rewrite_tag( '%' . WP_Service_Workers::QUERY_VAR . '%', '([^?]+)' );
}

The PWA plugin calls $wp_rewrite->wp_rewrite_rules() on init. If the option does not exist, it flushes the rewrite rules. If in the admin, this will cause 2 flushes because of the flush hooked into admin_init. That's why the quick fix would be to just call get_option( 'rewrite_rules') instead of $wp_rewrite->wp_rewrite_rules(). So just a 1 line change.

However, flushing like that should typically only occur on plugin activation/deactivation or when a new site is created in Multisite, or (if the plugin has that) in some sort of upgrade routine / migration script. That's how we do it in the Web Stories plugin for example: https://github.com/GoogleForCreators/web-stories-wp/blob/afe6e41af3f0086181a4cdf12060c2c2b6d4bf29/includes/Infrastructure/ServiceBasedPlugin.php#L116-L208

@westonruter
Copy link
Collaborator

Alternatively, we can avoid flushing the rules entirely because the plugin also contains this failsafe code which runs at the parse_query action:

// Handle case where rewrite rules have not yet been flushed.
if ( 'wp.serviceworker' === $wp->request ) {
$query->set( WP_Service_Workers::QUERY_VAR, 1 );
}

@lkraav
Copy link

lkraav commented Jan 5, 2024

So is there a code change planned?

@westonruter
Copy link
Collaborator

Yes. I need to set aside some time this month to do this and the 0.8 release.

@westonruter
Copy link
Collaborator

Here we go: #1096

@westonruter westonruter added this to the 0.8 milestone Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants