Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions packages/playground/website/playwright/e2e/opfs.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,39 @@ test('should start a new Playground after an initial OPFS sync was interrupted',
await website.waitForNestedIframes();
});

test('should not offer an unfinished initial OPFS sync loaded from storage as a recent autosave', async ({
website,
browserName,
}) => {
test.skip(
browserName !== 'chromium',
`This test relies on OPFS which isn't available in Playwright's flavor of ${browserName}.`
);

const interruptedSiteSlug = `interrupted-autosave-${Date.now()}`;
await website.page.goto(getTemporaryPlaygroundUrl());
await website.page.waitForFunction(() => !!navigator.storage?.getDirectory);
await writeInterruptedInitialOpfsSite(website.page, interruptedSiteSlug);

await website.page.goto('./');
const restoreNudge = website.page.getByLabel('Recent autosaved Playground');
const autosavedStatus = website.page.getByRole('button', {
name: 'Autosaved',
});
await expect(restoreNudge.or(autosavedStatus)).toBeVisible({
timeout: 120000,
});

expect(await restoreNudge.count()).toBe(0);
await expect(autosavedStatus).toBeVisible();
await expect
.poll(() => getActivePlaygroundSite(website.page))
.toMatchObject({
storage: 'opfs',
persistence: 'autosave',
});
});

test('should switch between sites', async ({ website, browserName }) => {
test.skip(
browserName !== 'chromium',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
} from './slice-ui';
import type { PlaygroundDispatch, PlaygroundReduxState } from './store';
import {
hasUnfinishedInitialOpfsSyncFromStorage,
isAutosavedSite,
isUnfinishedBlueprintRun,
selectSiteBySlug,
Expand Down Expand Up @@ -96,12 +97,9 @@ export function bootSiteClient(
return;
}
site = selectSiteBySlug(getState(), siteSlug) ?? site;
} else if (
site.loadedFromStorage === true &&
site.metadata.initialOpfsSyncPending === true
) {
// If the initial OPFS sync was interrupted, the site files are incomplete
// and we can't boot this site.
} else if (hasUnfinishedInitialOpfsSyncFromStorage(site)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I love this function name @ashfame. I know many people who would say it's too long and we should find a shorter one, but I really like it. It tells me everything I need to know about that operation without actually reading the function body ❤️

// The initial OPFS copy has not finished in storage, so the site files
// are incomplete and we cannot boot this site yet.
dispatch(
setActiveSiteError({
error: 'initial-opfs-sync-interrupted',
Expand Down
16 changes: 16 additions & 0 deletions packages/playground/website/src/lib/state/redux/site-lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,22 @@ export function isTemporarySite(site: SiteInfo) {
return !isStoredSite(site);
}

/**
* Indicates whether a site was loaded before its first OPFS copy completed.
*
* New stored Playgrounds keep this flag while their live client copies its
* initialized files. If another document loads the record before that flag is
* cleared, the files are not yet bootable. The creating document may still be
* syncing or may have stopped.
*/
export function hasUnfinishedInitialOpfsSyncFromStorage(site: SiteInfo) {
return (
site.loadedFromStorage === true &&
isOpfsBackedSite(site) &&
site.metadata.initialOpfsSyncPending === true
);
}

/**
* Indicates whether a site has durable storage. Autosaved sites are stored but
* not explicitly saved, so callers that need user-pinned sites use
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,16 +111,27 @@ describe('stored sites', () => {
vi.doUnmock('./store');
});

it('classifies a normal autosave but not an unfinished Blueprint run as restorable', async () => {
it('only classifies bootable autosaves as restorable', async () => {
const { isRestorableAutosavedSite } = await import('./slice-sites');
const autosave = createSiteInfo({ slug: 'autosave' });
autosave.metadata.persistence = 'autosave';
const inProgressAutosave = createSiteInfo({ slug: 'in-progress' });
inProgressAutosave.metadata.persistence = 'autosave';
inProgressAutosave.metadata.initialOpfsSyncPending = true;
const storedUnfinishedAutosave = createSiteInfo({
slug: 'stored-unfinished',
});
storedUnfinishedAutosave.loadedFromStorage = true;
storedUnfinishedAutosave.metadata.persistence = 'autosave';
storedUnfinishedAutosave.metadata.initialOpfsSyncPending = true;
const unfinishedRun = createSiteInfo({ slug: 'unfinished-run' });
unfinishedRun.metadata.persistence = 'autosave';
unfinishedRun.metadata.siteSlugToReturnToIfBlueprintFails =
'source-site';

expect(isRestorableAutosavedSite(autosave)).toBe(true);
expect(isRestorableAutosavedSite(inProgressAutosave)).toBe(true);
expect(isRestorableAutosavedSite(storedUnfinishedAutosave)).toBe(false);
expect(isRestorableAutosavedSite(unfinishedRun)).toBe(false);
});

Expand Down
14 changes: 10 additions & 4 deletions packages/playground/website/src/lib/state/redux/slice-sites.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import { deriveSlugFromSiteName, getUniqueSiteSlug } from './site-slug';
import {
getAutosavedSitesToPrune,
getSitesSortedByRecency,
hasUnfinishedInitialOpfsSyncFromStorage,
isAutosavedSite,
isStoredSite,
isTemporarySite,
Expand All @@ -53,6 +54,7 @@ export {
getSiteRecencyTimestamp,
getSitesSortedByRecency,
getSitePublicPersistence,
hasUnfinishedInitialOpfsSyncFromStorage,
isAutosavedSite,
isExplicitlySavedSite,
isOpfsBackedSite,
Expand Down Expand Up @@ -408,15 +410,19 @@ export function pruneAutosavedSites(
/**
* Checks whether a stored Playground may be offered as an autosave.
*
* A Blueprint run uses autosave persistence before its initial OPFS copy
* succeeds. Its return target marks it unfinished, so callers withhold it from
* Recent and matching-setup restore suggestions until that copy succeeds.
* An unfinished first OPFS copy loaded from storage cannot boot yet. A
* Blueprint run also uses autosave persistence before that copy succeeds.
* Callers withhold both from Recent and matching-setup restore suggestions.
*
* @param site The Playground whose autosave lifecycle should be checked.
* @returns Whether the Playground is a restorable autosave.
*/
export function isRestorableAutosavedSite(site: SiteInfo) {
return isAutosavedSite(site) && !isUnfinishedBlueprintRun(site);
return (
isAutosavedSite(site) &&
!hasUnfinishedInitialOpfsSyncFromStorage(site) &&
!isUnfinishedBlueprintRun(site)
);
}

/**
Expand Down
Loading