-
Notifications
You must be signed in to change notification settings - Fork 110
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
Add optimization_detective_disabled query var to disable behavior #1193
Add optimization_detective_disabled query var to disable behavior #1193
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
@joemcgill What do you think of this being a common pattern we use across other Performance Lab plugins? Being able to disable the functionality of a plugin in the wild could be a great way to compare how effective it is at improving performance. Naturally this wouldn't make sense for core, but for a feature plugin it could be valuable. |
Are you proposing that we would be able to disable all of our feature plugins by passing a query param for each feature, or are you thinking of a single param that would disable all of the features? I guess as a way to collect some A/B data, this could be useful, though we wouldn't really be able to use those params in collected user metrics, like CrUX. Also, something to keep in mind is that even though most full-page caches are likely already handling GET requests with query params separately, there may be some where the addition of the query param may still result in a previous cached result being served. |
@joemcgill I was thinking each feature would have their own query parameter, so we can granularly turn off optimizations to see their relative impact. This would only help in a lab context, as you note, as we wouldn't have field metrics for any A/B comparison. And yes, full page caches may interfere with this, so when attempting to disable a feature we'll need to make sure that it is actually disabled when we load the page. |
Makes sense to me. I think this is one of those cases where having a single implementation that automatically gets incorporated into all our plugins would be useful. We don't currently have anything like that set up for this repo but maybe it's time to set that up so we can handle use cases like this one and the need to support embedded plugins, like #1081. What do you think? |
@joemcgill Yeah, I was thinking about something like that too. However, doing something as low level as #1081 may be too low. A plugin may have multiple aspects to the optimizations it performs (e.g. fetchpriority and lazy loading) so it would be useful to be able to target each feature discretely, rather than be able to turn off the entire plugin altogether. So this makes me think more like "feature flag" query parameters are the most useful here. Given that each plugin will have their own set of features, perhaps it doesn't make sense to have a unified API for managing them? Or perhaps it could just be a common query var that we use, for example:
Then |
I like the idea of a common pattern for feature flags. Let's try to be observant of when we see this type of pattern popping up in multiple plugins and open a new issue for consolidating logic/architecture when it makes sense. |
Fixes #1069
This will allow us to add
?optimization_detective_disabled
to any URL running with Optimization Detective in order to generate the page without the optimizations. This will be useful for us to observe in the wild the performance gains by comparing a page with the non-optimized version.