Skip to content

Conversation

@tjwiebell
Copy link
Contributor

Description

Outlined in issue #52, resolvers were lacking caching headers, which was in turn causing some down stream issues in Fastly around text encoding/compression; which was dinging us on Lighthouse. I do think we should leave the issue open and have a general conversation about caching, because this issue is cross platform and also affects upward-js, but this resolve some Magento Cloud specific issues in the meantime.

As a starting point for, we're going to make the assumption that all static assets should be cached, and just hard-code this header into the Directory resolver.

Related Issue

Acceptance

Verification Stakeholders

Specification

Verification Steps

  1. Deploy to Magento Cloud (will require feature/upward-dependency-testing for magento/module-upward-connector dependency)
  2. With Developer Console open, clear cache and open the app
  3. Filter for a static asset like client-<hash>.js or vendor-<hash>.js
  4. Verify Cache-Control header has a max-age value > 0
  5. Verify Content-Encoding header value is gzip

Screenshots / Screen Captures (if appropriate)

Checklist

  • I have updated the documentation accordingly, if necessary.
  • I have added tests to cover my changes, if necessary.

Copy link

@sirugh sirugh left a comment

Choose a reason for hiding this comment

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

LGTM!

@devpatil7 devpatil7 merged commit 8a4ae61 into develop Dec 16, 2019
@devpatil7 devpatil7 deleted the tommy/cache-control branch December 16, 2019 19:12
@opepin
Copy link

opepin commented Feb 24, 2020

hello,
After upgrading to 1.1.2, I'm getting the new cache header from the directory
cache-control: max-age=31557600

but also some conflicting cache headers that may mess up some web browsers/proxy
expires: Thu, 19 Nov 1981 08:52:00 GMT
pragma: no-cache

This headers are coming from PHP session cache limiter. There is no need for a session to serve static resources in Upward .
Is there a way to disable Magento session creation in that scope ?

this is related to some issues with web api magento/magento2#26032

A workaround could be to force the session cache limiter to public for Directory Resolver to override those headers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants