Flushing the rewrite rules is "wrong" on the front-end

[azaozz]

Follow-up from #44142.

"Flushing" these rules takes a lot of resources, writes to the db, and (usually) overwrites .htaccess. Because of that it should only be performed in wp-admin, especially in production. Add a _doing_it_wrong() notice so theme and plugin authors are aware if this.

[swissspidy]

This will create lots and lots of notices. Many plugins and themes flush rewrite rules on init, because they got that from some tutorial or something.

Such a change should get in as early as possible.

[desrosj]

What if we started with a post on Make Core that explains the reasons why it is wrong to flush on the front end, and details the correct way to perform this action? We could also state in that post that it will be included in 5.0, and we could post that any time.

[kraftbj]

Sample first run:
Rewrite rules should only be flushed within wp-admin

In 5.0 via #44171, rewrite rules can only be flushed within an administrative context (wp-admin).

Processing rewrite rules, generally, is an expensive operation, which is why WordPress stores a compiled set of rewrite rules in the database. When plugins or themes flush the rewrite rules on the front-end, this requires the site to perform the work of compiling the rewrite rules, performing two writes to the database, and, by default, updating the .htaccess or web.config to continue loading the site.

At best, this increases the site load time for visitors, but at worst, can overload the database and bring the site down depending on the number of concurrent visitors.

Starting in 5.0, a _doing_it_wrong notice will occur and the flush_rewrite_rules() will return early without action if is_admin is false. Ideally, flushing the rewrite rules should only occur when there has been a preceding action impacting rewrite rules, such as plugin or theme activation/deactivation. If actively developing a site, plugin, or theme, add_action( 'admin_init', 'flush_rewrite_rules' ); is acceptable. This is often best done via a dedicated development plugin to avoid accidentally shipping it in production code.

[kraftbj]

Props azaozz for the code, kraftbj for the location. Carried over from #44142.

[kraftbj]

Another kicker: WP CLI does not pass is_admin() == true. Should we have a filter that is heavily documented that it should only be used by WP CLI or in similar contexts where allowing non-admin writes is understood and acceptable?

[iandunn]

Replying to desrosj :

We could also state in that post that it will be included in 5.0, and we could post that any time.

I'm wondering if it'd be best to post that as soon as possible (after this is committed), to give devs plenty of lead time? Many will already have a lot of work to do around Gutenberg, so spreading it out as much as possible will help minimize the pain.

If it is published soon, though, it'd be good to include a reminder in a 5.0 dev note round-up.

Replying to kraftbj:

Should we have a filter that is heavily documented that it should only be used by WP CLI or in similar contexts [...]?

Would it be better to just change the condition to if ( ! is_admin() && 'cli' !== php_sapi_name() ) { ?

[kraftbj]

Replying to iandunn:

Would it be better to just change the condition to if ( ! is_admin() && 'cli' !== php_sapi_name() ) { ?

I don't see any other times in Core that we check for CLI (using that or ( defined( 'WP_CLI' ) && WP_CLI ).

I think it would be fine to add that, but are we good with adding the first CLI check to Core?

[desrosj]

I like the first pass, @kraftbj. I can copy it over to Make if we are all comfortable with this plan.

Should we also bring this up in the Core Dev Chat this week for feedback?

[iandunn]

Adding the CLI check seems appropriate to me in this case, but I'd definitely like to hear what others think. I don't think we should test for WP-CLI specifically, though, since there are other CLI scripts that do similar things.

[pento]

The number of warnings this will generate makes me nervous. 🙂

It seems like we could try some outreach to popular plugin/theme authors to get them to fix this behaviour, before we start enforcing it.

[kraftbj]

What's the best way to identify some of these plugins or themes that do that? I'd like to move forward with this so we hopefully add it sometime in 2019.