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

Only set long expiration for files in _app/immutable directory #29

Merged
merged 2 commits into from
Jul 5, 2022

Conversation

sfriedel
Copy link
Contributor

Recently SvelteKit introduced the _app/immutable directory in the buildout. This directory contains all files that should be cached with a long TTL. Serving the whole _app/ directory with a long TTL impacts the SvelteKit update check via the updated store from $app/stores (see: https://kit.svelte.dev/docs/modules#$app-stores-updated) which relies on being able to fetch the current app version from _app/version.json.

This PR splits the handlers for _app/ and _app/immutable and only sets the 30d expiration on the _app/immutable handler. Other assets in _app/ are then served with the default expiration which can further be customized by setting default_expiration in the top-level app.yaml.

Similar changes in the default adapters were introduced in sveltejs/kit#5051

@HalfdanJ
Copy link
Owner

With #28 merged, can you try and rebase / merge to master. Also, integration test needs to be updated to contain the new expected yaml in /tests/expected_app.yaml. Try running npm run test:integration

@sfriedel
Copy link
Contributor Author

sfriedel commented Jul 5, 2022

HI @HalfdanJ and sorry I completely missed the notifications on the PRs. I see you already merged the other one. I'll rebase this one and update the integration tests.

@sfriedel
Copy link
Contributor Author

sfriedel commented Jul 5, 2022

@HalfdanJ I updated /tests/expected_app.yaml. The integration tests are still failing but that seems unrelated to the changes in this branch because it also happens in main. I opened #32 which seems to be the root cause for the integration tests failures.

@HalfdanJ HalfdanJ merged commit 3bd4960 into HalfdanJ:main Jul 5, 2022
@HalfdanJ
Copy link
Owner

HalfdanJ commented Jul 5, 2022

Thanks @sfriedel !

@sfriedel sfriedel deleted the expiration-for-immutable-only branch July 6, 2022 09:13
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