Skip to content

introduce woocommerce_enable_nocache_headers filter #26802

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.

Sign up for GitHub

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

Jump to bottom
Merged
rrennick merged 2 commits into from
Jun 23, 2020
Merged

introduce woocommerce_enable_nocache_headers filter #26802

rrennick merged 2 commits into from
Jun 23, 2020

Conversation

@rrennick
Copy link
Contributor

@rrennick rrennick commented Jun 17, 2020 •
edited by Konamiman
Loading

All Submissions:

Changes proposed in this Pull Request:

This PR adds a woocommerce_enable_nocache_headers filter to allow CDN plugins to disable adding the WC nocache headers.

Closes # .

How to test the changes in this Pull Request:

  1. Open the network tab in your browser console.
  2. Navigate to the shop page.
  3. Check the headers for cache-control: no-transform, no-cache, no-store, must-revalidate
  4. Add the filter add_filter( 'woocommerce_enable_nocache_headers', '__return_false' );
  5. Reload the page
  6. Check the headers for cache-control: no-cache, no-store, must-revalidate

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Changelog entry

Dev - Add filter to allow disabling nocache headers.

@rrennick rrennick added Enhancement The issue is a request for an enhancement. status: needs review labels Jun 17, 2020
@lkraav
Copy link
Contributor

lkraav commented Jun 18, 2020 •
edited
Loading

It still doesn't explain why there's a misleading core comment

// no-transform: Opt-out of Google weblight if page is dynamic e.g. cart/checkout. https://support.google.com/webmasters/answer/6211428?hl=en.

But there's no actual if code branch, and it's applied sitewide.

@rrennick
Copy link
Contributor Author

rrennick commented Jun 18, 2020

It still doesn't explain why there's a misleading core comment

@lkraav Thanks, nice catch.

@rrennick
Copy link
Contributor Author

rrennick commented Jun 18, 2020

But there's no actual if code branch, and it's applied sitewide.

The nocache_headers filter runs before WP_Query has parsed the query. To make this conditional we would need to duplicate the WP_Query logic. If the WP core filter was moved to after the query was parsed that would make conditional logic more practical. I searched recent core track and didn't see a request for that.

claudiosanches
claudiosanches approved these changes Jun 23, 2020
View reviewed changes
Copy link
Contributor

@claudiosanches claudiosanches left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@woocommercebot woocommercebot added status: approved and removed Enhancement The issue is a request for an enhancement. status: needs review labels Jun 23, 2020
@rrennick rrennick merged commit e9f71ac into master Jun 23, 2020
@rrennick rrennick deleted the fix/25833 branch June 23, 2020 16:35
@woocommercebot woocommercebot added release: add changelog Mark all PRs that have not had their changelog entries added. [auto] release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Jun 23, 2020
This was referenced Jun 23, 2020
Closed
Merged
@Konamiman Konamiman added this to the 4.4.0 milestone Jul 27, 2020
@tammullen tammullen removed the release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] label Jul 28, 2020
@Konamiman Konamiman removed the release: add changelog Mark all PRs that have not had their changelog entries added. [auto] label Jul 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@claudiosanches claudiosanches claudiosanches approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

4.4.0

Development

Successfully merging this pull request may close these issues.

7 participants

@rrennick @lkraav @claudiosanches @Konamiman @tammullen @woocommercebot