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

add option to configure when shoebox data expires #369

Merged
merged 2 commits into from
Jun 18, 2020
Merged

add option to configure when shoebox data expires #369

merged 2 commits into from
Jun 18, 2020

Conversation

st-h
Copy link
Contributor

@st-h st-h commented Jun 16, 2020

In certain cases, like the browser restoring its session, a cached version of the index.html page may be served by the browser, which contains shoebox data from the initial, cached request. The adapter is not aware that this data is stale, and outdated information is displayed to the user (without ever making a request to the backend). To prevent that, this PR adds a param to configure after which duration data no longer is served from the shoebox.

// config/environment.js
ENV = {
  storefront: {
    maxAge: 10 // shoebox expires 10 minutes after the fastboot server has rendered the page
  }
}

If this value is not present in config/environment.js, current behaviour is maintained: the shoebox does never expire.

The current date when the adapter is initialised is stored in the shoebox and is compared when rendered in the browser. This should be fine, unless the fastboot server is optimised to not tear down the adapter across different requests.

I also removed the ember-cli-htmlbars-inline-precompile dependency, as it is no longer needed:

DEPRECATION: ember-cli-htmlbars-inline-precompile is no longer needed with ember-cli-htmlbars versions 4.0.0 and higher, please remove it from `package.json

I tried to add some tests, but could not find a way to mock the config with the current test setup. If someone can provide a hint how to make that work, I'll give it another try.

@ryanto
Copy link
Member

ryanto commented Jun 17, 2020

Wow nice find! I'm all for this, just two quick questions:

I know a lot of folks cache their fastboot responses, and I don't think the storefront shoebox is the right place to "bust" that cache. However, it sounds like this is mainly for client side caches where the browser doesn't re-fetch or re-validate index.html?

Also, I know we empty the shoebox once we've queried it, by deleting the html/script tags, since it's only useful for the initial load. However, it sounds like those deletes don't survive the browser restoring?

👍

@st-h
Copy link
Contributor Author

st-h commented Jun 17, 2020

Backstory: We have received complaints from a customer, who would turn his computer off at night and come back in the morning, starting the computer, opening firefox, which would restore its session, often with our app in the visible tab. The page would sometimes display stale data, requiring a reload of the page. I've tried to debug the firefox restore behaviour and it looks like when the page is the one in the currently visible tab, it is pretty aggressively cached. We serve the index page providing the header Cache-Control: max-age=60, public, however the restore mechanism seems to ignore that and serve the cached page, ignoring how old it might be.

When the page is restored in another tab, the header seems to be honoured and things look as they would work correctly.

@ryanto I am not sure if I understand your first question correctly. This is not about additionally caching fastboot responses. This is about the browser serving an outdated page, which contains outdated shoebox data. Currently there is now way to know how old the data in the shoebox is and so this addon would silently serve the data from the outdated shoebox as current data. If all browsers would work fine in all situations there pretty would not be no need for such a mechanism. I have created a Firefox bug report a few days ago, but haven't received any response so far. (this might even be a feature to restore the visible page faster)

This PR keeps the behaviour in terms of deleting the shoebox content. In case the data in the shoebox is too old, it is deleted without being served to the app. However, this mechanism did not help to solve this issue, as the outdated data is served from the shoebox and the ember app goes:
"ah, nice some data, thanks. I'll just render that. Now that I have all data, there is nothing more to do."
If the app fully makes use of fastboot, it pretty much loads silently without ever connecting to the server, as all its required data is in the shoebox. The browser serves the initial page from the request, not the one which has been modified while the app loads in the browser, therefore the initial shoebox is in place.

The firefox session restore is the only place where I have observed such behaviour. However, I haven't checked if other browser might do something similar in certain situations, though. But likely it is a good idea to make sure the data in the shoebox is ignored, if it is possibly stale. Fixing it here in the addon seemed the best place, as it is as close to the root of the issue as possible, as we can't control or double check what the browser is doing.

@ryanto
Copy link
Member

ryanto commented Jun 18, 2020

Awesome, thanks for the answers.

@ryanto ryanto merged commit 0f0e871 into embermap:master Jun 18, 2020
@ryanto
Copy link
Member

ryanto commented Jun 18, 2020

Published as v0.17.3

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.

2 participants