Skip to content
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

Conflict with Super Cache plugin #188

Closed
iandunn opened this issue Jul 3, 2019 · 5 comments · Fixed by #189
Closed

Conflict with Super Cache plugin #188

iandunn opened this issue Jul 3, 2019 · 5 comments · Fixed by #189
Labels
bug Something isn't working
Milestone

Comments

@iandunn
Copy link
Collaborator

iandunn commented Jul 3, 2019

I haven't had a chance to figure out exactly what's causing this yet, so this may be invalid, but I've noticed that sites with the pwa plugin active seem to disable WP Super Cache by making it think that the current visitor is logged in.

To reproduce:

  1. Open an Incognito window
  2. Open the network dev tool
  3. Visit https://2019.europe.wordcamp.org/contact/
  4. View the response for the /contact/ request, and you'll see <!-- Cached page generated by WP-Super-Cache on 2019-07-04 00:51:12 --> at the bottom, which is expected. At this point the service worker hasn't been installed yet.
  5. Refresh the page now that the service worker is installed, and you'll see <!-- WP Super Cache: Caching disabled for known user. User logged in or cookie found. --> at the bottom of the response.

If you curl https://2019.europe.wordcamp.org/contact/ you'll always get the correct cached response. If you repeat the steps above with a site that doesn't have pwa activated -- e.g., 2019.us -- then you'll also always see the correct cached response.

@iandunn iandunn added the bug Something isn't working label Jul 3, 2019
@westonruter
Copy link
Collaborator

Ah, interesting. The PWA plugin does set a cookie when the service worker is installed:

https://github.com/xwp/pwa-wp/blob/b0dcc2d668cd3dbb410af89b9af45e548436e73e/wp-includes/service-workers.php#L161-L165

And then WP-Super-Cache is doing:

	} elseif ( $wp_cache_not_logged_in == 1 && ! empty( $_COOKIE ) ) {
		wp_cache_debug( 'wpsc_is_caching_user_disabled: true because cookie found' );
		return true;

The cookie is being used to prevent script concatenation:

https://github.com/xwp/pwa-wp/blob/b0dcc2d668cd3dbb410af89b9af45e548436e73e/wp-includes/service-workers.php#L261-L268

The intention here is that the service worker will precache the scripts being used in the admin so that they will be served immediately form the browser cache and thus there is no need to concatenate. This intention may be misguided.

Nevertheless, what is for sure is that this only applies to the admin. Script concatenation via load-scripts.php is not a frontend thing. Therefore, there the PWA plugin should avoid setting this cookie for the front service worker. It should be restricted to the admin service worker only, at least.

This will also improve frontend performance a bit by not having this extra cookie being sent in all requests.

@westonruter
Copy link
Collaborator

That being said, should WP-Super-Cache really be preventing any page caching when there is any cookie present? That seems like it would cause caching to be disabled very often.

@westonruter
Copy link
Collaborator

For example, doesn't the ubiquitous wordpress_test_cookie cookie cause caching to be disabled in WP-Super-Cache?

@westonruter
Copy link
Collaborator

Here's a fix: #189.

@iandunn
Copy link
Collaborator Author

iandunn commented Jul 4, 2019

Ah, you're right: https://github.com/Automattic/wp-super-cache/issues/620

I'll try to test #189 tomorrow, thanks!

@westonruter westonruter added this to the 0.3 milestone Jul 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants