Skip to content

Conversation

@crazytonyli
Copy link
Contributor

What?

Perform the cleanup step before the download assets step.

Why?

The cleanup step removes the assets that were just downloaded if the previous function call was one day ago.

To reproduce the issue locally:

  1. change the interval from 1 day to 1 second.
  2. Go to the Demo app.
  3. Add a new site.
  4. Tap the "Prepare Editor" button to preload.
  5. Open the editor on the site. You'll see the editor open slowly because it's downloading assets.

// Cleanup errors are intentionally ignored so that the download process
// continues uninterrupted. Failures are expected when running for the
// first time, since the storage directories may not yet exist.
await onceEvery(.seconds(86_400)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it may be worth reconsidering the "once every x interval" implementation.

The "handle" of this timestamp is static, which means one timestamp applies to all sites. At the bare minimum, the timestamp should be stored per site (configuration.siteURL).

Since we'll be supporting multiple post types, the number of handles will be N(sites) x N(post types). I don't feel like UserDefaults is the right place to store that data.

What do you think about using the last modified date of the manifest.json as the timestamp, so that we don't need to store any more timestamps anywhere?

@dcalhoun
Copy link
Member

dcalhoun commented Feb 9, 2026

The cleanup step removes the assets that were just downloaded if the previous function call was one day ago.

I struggle to understand the problem we aim to solve with these changes. Following the reproduction steps did not help me either; I always saw the activity indicator, not the progress bar. Would be willing to elaborate on the observed problem please?

@crazytonyli
Copy link
Contributor Author

@dcalhoun Sorry, I misunderstood the code. I double checked. There is no issue with the current implementation.

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