#30501 closed defect (bug) (fixed)
Prevent flushing rewrites during page load
Reported by: | joostdevalk | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | 4.2 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Permalinks | Keywords: | 4.2-early has-patch commit |
Focuses: | Cc: |
Description
Flushing rewrites during the pageload breaks a lot of plugins. They might not have registered their rewrite yet which would cause issues. I propose preventing the flushing from working and forcing it to be run on shutdown instead.
Attachments (6)
Change History (40)
This ticket was mentioned in Slack in #core by joostdevalk. View the logs.
10 years ago
This ticket was mentioned in Slack in #core by joostdevalk. View the logs.
10 years ago
#4
@
10 years ago
I'm +1 on adding this, for sure. I don't think there's any issues with changing this that I can see.
(Note that this solves a different problem to the one I'd discussed previously in ticket:29118#comment:5: I was aiming to avoid ever calling flush rewrites, whereas the problem here is the infamous flush-before-registration.)
This ticket was mentioned in Slack in #core by joostdevalk. View the logs.
10 years ago
This ticket was mentioned in Slack in #core by joostdevalk. View the logs.
10 years ago
This ticket was mentioned in Slack in #core by joostdevalk. View the logs.
10 years ago
#10
@
10 years ago
Just updated the patch to check for did_action( 'init' )
so you can't flush too early anymore, but the flush would still work on the current pageload.
#11
@
10 years ago
@joostdevalk, how about pushing this out of init
, perhaps to wp_loaded
? I can imagine plenty of plugins may register rewrites after priority 10 of init
, and this patch wouldn't solve this problem for them. wp_loaded
is the last action that fires before WP::parse_request()
(where the rewrite rules are used), so it seems like a decent time to regenerate should that be requested.
#13
@
10 years ago
- Keywords 4.2-early added
rmccue approves! If we're going to do this (and we should), let's try and do it early in 4.2 so that we find any plugin breakages as soon as possible.
This ticket was mentioned in Slack in #core by joostdevalk. View the logs.
10 years ago
#19
@
10 years ago
I am not sure if I understand it well, please see example code:
function some_rewrite() { global $wp_rewrite; $wp_rewrite->author_base = 'something'; $wp_rewrite->flush_rules(); } add_action('init', 'some_rewrite');
Calling $wp_rewrite->flush_rules()
is wrong here because it is called during every page load. What is the right way for users when they want to change author to something else? And they need only quick hack for functions.php
(and not flushing rewrite rules during plugin activation).
I saw many times this piece of code. It will work in 4.2 and only running $wp_rewrite->flush_rules()
function will be automatically moved to wp_loaded
?
#20
follow-up:
↓ 21
@
10 years ago
Flushing on every page load is still bad and this patch does not prevent that. What this DOES prevent is a plugin or theme running flush_rewrite_rules before plugins have had a chance to register their rewrites, which causes URLs to break. So it doesn't prevent it from running, it just moves it.
#21
in reply to:
↑ 20
;
follow-up:
↓ 22
@
10 years ago
Replying to joostdevalk:
Flushing on every page load is still bad and this patch does not prevent that. What this DOES prevent is a plugin or theme running flush_rewrite_rules before plugins have had a chance to register their rewrites, which causes URLs to break. So it doesn't prevent it from running, it just moves it.
Is add_action
smart enough to call flush_rewrite_rules
only once vs the n amount of calls various plugins will do?
#22
in reply to:
↑ 21
@
10 years ago
Replying to Funkatronic:
Is
add_action
smart enough to callflush_rewrite_rules
only once vs the n amount of calls various plugins will do?
Yes.
#23
follow-up:
↓ 24
@
10 years ago
- Resolution set to fixed
- Status changed from accepted to closed
[31104] seems to work as expected.
#24
in reply to:
↑ 23
@
10 years ago
Replying to SergeyBiryukov:
[31104] seems to work as expected.
Is there any reason for dumping the $hard
argument? From the looks of the changeset, if you try to do a soft flush before wp_loaded
, you'll end up doing a hard one later on instead.
#26
@
10 years ago
A possible workaround off the top of my head:
static $do_hard_later; if ( ! did_action( 'wp_loaded' ) ) { add_action( 'wp_loaded', array( $this, 'flush_rules' ) ); $do_hard_later ||= $hard; return; } elseif ( isset( $do_hard_later ) && doing_action( 'wp_loaded' ) ) { $hard ||= $do_hard_later; $do_hard_later = null; }
#27
@
10 years ago
- Keywords needs-patch added; has-patch commit removed
@Denis-de-Bernardy: Would you care to follow up with a patch? It seems solving the $hard
issue is the only thing preventing this ticket from being resolved in 4.2.
#28
@
10 years ago
30501.patch should take care of the $hard
issue.
#30
@
10 years ago
30501.2.patch doesn't account for a situation where flush_rules()
is called with $hard = true
and then with $hard = false
. It would only do a soft flush in that case.
30501.3.patch fixes that.
#32
@
10 years ago
please, tell the world (all plugins developers, in a big RED NOTICE, that they should disable flushing on every page load...
they'd better to use smth like this:
if (get_option($postype . "_checkpoint_12345")) { ....flush... ....delete_option($postype . "_checkpoint_12345");........ }
Related to but not the same as #29118.