Make WordPress Core

Opened 5 years ago

Last modified 3 years ago

#47686 new defect (bug)

Always flush rewrite rules when active plugins changes & on plugin upgrades

Reported by: johnjamesjacoby's profile johnjamesjacoby Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Rewrite Rules Keywords:
Focuses: Cc:

Description

I think it's odd that plugins each need to roll their own rewrite rule flushing.

It's definitely nice to have the freedom to decide when it's necessary, but it is inconvenient as a developer to reinvent a new way with every new plugin whenever a rule needs to be changed.

It's also inconvenient to users when a plugin forgets to flush rules, or when a developer is unaware that it is their responsibility to flush their own rules.

I'm proposing here that WordPress core could improve the lives of both developers and users by automatically flushing rewrite rules every time a plugin is activated, deactivated, or updated.

For context, flush_rewrite_rules() is already called inside of check_theme_switched() which in hooked to init and always flushes rewrite rules when a new theme is activated, and I'm suggesting we consider doing this for plugins too.

Change History (10)

#1 @Ipstenu
5 years ago

This could be problematic with caching systems, many of which are built to flush cache if permalinks are flushed.

People upgrade and activate/deactivate plugins far more often than themes. And not all plugins need to flush permalinks to work. In fact, I'd wager that most plugins don't - though that's an anecdotal assumption based on the reviews I've done.

#2 follow-up: @johnjamesjacoby
5 years ago

This could be problematic with caching systems, many of which are built to flush cache if permalinks are flushed.

That's interesting. It's a pretty bad assumption, but I can see how people could make it.

Neither flush_rewrite_rules() nor $wp_rewrite->flush_rules() have their own hooks, so I assume they are hooking into one of the hooks inside of update_option()?

Since #29107, $wp_rewrite->flush_rules() now does a double update - once to empty, another to the new rules. If that commit didn't break caching plugins (or nobody noticed, or cared enough to say so) I'd be surprised if my suggestion would cause any more harm.

If caching plugins are hooking into updated_option or update_option_rewrite_rules, and if we can confirm with unit tests that core isn't over flushing when the values are updated (see: #38903) then it should still be viable.

Even if it's not yet, I still think it's worth it to the developer community and the user community to do more research into how rewrite rule flushing could be done automatically when it's needed, and not done repeatedly if it's not.

#3 @johnjamesjacoby
5 years ago

Thinking out loud, one approach could be to store a shadow option of what the rewrite rules were and what WordPress thinks they are after an environment change (theme or plugin change, etc...) compare the two, and then WordPress will internally always flush when they are different (a lot like how $wp_query and $wp_the_query comparisons work.)

Page load to page load, like @Ipstenu said, rewrite rules are unlikely to require a flush. But... it is common for plugins to both overflush and forget to flush. The above idea would basically make it a non-issue, and nobody would ever need to call it themselves directly ever - WordPress would just know. Theoretically.

#5 @SergeyBiryukov
5 years ago

  • Component changed from General to Rewrite Rules

#6 @johnjamesjacoby
5 years ago

  • Component changed from Rewrite Rules to General

Maybe core doesn't even need to store the second option to compare.

Maybe... the first option is actually what's queried early by wp_load_alloptions(), and the second option is what WordPress thinks they should be on the wp_loaded hook (where all calls to flush_rewrite_rules() get deferred to, thanks to the $do_hard_later handler.

Hmmmmm.

#7 @johnbillion
5 years ago

Bear in mind that the flushing needs to happen late on the subsequent request to ensure that all custom post types, taxonomies, rewrite endpoints, and custom rewrite rules have been registered before the flush happens.

#8 @SergeyBiryukov
5 years ago

  • Component changed from General to Rewrite Rules

#9 in reply to: ↑ 2 @kevindees
5 years ago

I added a working implementation of this idea as tagged by @SergeyBiryukov #47526

Replying to johnjamesjacoby:

This could be problematic with caching systems, many of which are built to flush cache if permalinks are flushed.

That's interesting. It's a pretty bad assumption, but I can see how people could make it.

Neither flush_rewrite_rules() nor $wp_rewrite->flush_rules() have their own hooks, so I assume they are hooking into one of the hooks inside of update_option()?

Since #29107, $wp_rewrite->flush_rules() now does a double update - once to empty, another to the new rules. If that commit didn't break caching plugins (or nobody noticed, or cared enough to say so) I'd be surprised if my suggestion would cause any more harm.

If caching plugins are hooking into updated_option or update_option_rewrite_rules, and if we can confirm with unit tests that core isn't over flushing when the values are updated (see: #38903) then it should still be viable.

Even if it's not yet, I still think it's worth it to the developer community and the user community to do more research into how rewrite rule flushing could be done automatically when it's needed, and not done repeatedly if it's not.

Last edited 3 years ago by SergeyBiryukov (previous) (diff)

#10 @ocean90
4 years ago

#51903 was marked as a duplicate.

Note: See TracTickets for help on using tickets.