Skip to content

Ask the drive for its apps again when a view menu opens (#1846) - #1904

Closed
michielbdejong wants to merge 1 commit into
developfrom
claude/drive-apps-menu-refresh
Closed

michielbdejong wants to merge 1 commit into
developfrom
claude/drive-apps-menu-refresh

Conversation

@michielbdejong

Copy link
Copy Markdown
Contributor

Refs #1846. Follow-up to #1847, which only partly fixed the flaky money.spec.ts:220 in ontola/atomic-plugins ("Add view" → menuitem 'New app' times out after 10 s, on both attempts).

What the traces show

Traces from atomic-plugins runs 36584202173 and 36585675282, at candidate14 1432e24:

  • The page reloads onto the table (page.goto(showUrl(...))). The OPFS ClientDb is ready about 1 s later, and "+ Add view" opens about 0.5 s after that. The menu has no app in it.
  • After the reload there is no /query?property=isA&value=<app class> request, so the app list came from the local OPFS query.
  • About 0.5 s after the menu opens, a ws-sync-push arrives for the app, its agent and its entry point, and each logs incomplete Loro import … missing base state, fetching a full snapshot. The page's in-memory copy did not have the history that the delta was built on. That copy was either not loaded yet, or it was hydrated from query JSON, which builds a Loro doc from fresh ops. The store then keeps the resource as a placeholder under repair.
  • The menu never gets the app in the next 10 s.

Why the hook missed it

useDriveApps from #1847:

  1. attached its listeners only after findSchema resolved;
  2. ignored events for resources that were loading;
  3. held back every live event (if (queried) publish()) until the mount chain finished: findSchema, then the local query, then an awaited store.getResource(member) for each app, one at a time.

getResource waits up to 10 s for a loading or recovering resource, which is exactly the test's click budget. Measured locally with timestamps (Vite dev, no load), the chain published 1 to 1.9 s after the schema lookup, and every time that was after the menu had already opened. With the repair placeholder in the way it gets much slower.

Fix

  • Attach listeners first. Fold every answer in as soon as it arrives, and read the live store resource each time, so an older answer can't overwrite a newer one. The decided bookkeeping is no longer needed.
  • Read query members in parallel. A slow member no longer blocks the others or the live events.
  • useDriveApps now returns { apps, refresh }. The "+ Add view" menu and the view-tab menu ("View type") call refresh() from bindActive(true). Opening the menu re-runs the query and re-reads every app the hook already knows about, so a resource whose state changed without an announcement still shows up.

Tests

In useDriveApps.live.test.tsx (jsdom, real Store), two new cases. Both fail on the #1847 hook:

  • lists, on refresh, an app the store took in without announcing it: the app goes into the store and the local index with no event. The menu opens and the app is listed.
  • shows the other apps while one of them is still being repaired: one member is a loading placeholder that getResource waits on. The other apps show within 1 s, and the repaired app joins when the store announces it.

vitest src/chunks/AppPage src/chunks/TablePage (453 tests), tsc, oxlint and oxfmt all pass.

I did not add an e2e test. A Playwright version of the money flow (create table, app with renders set, reload, Add view) passed 30+ times locally on the old hook. It only fails when the incomplete sync push races the menu, and I couldn't reproduce that race outside CI.

Not in this PR

The deeper store issue: a ws-sync-push delta for a subject that is in OPFS but not yet in memory (or only hydrated from query JSON) is imported into a doc without that history, so the import is incomplete. That needs its own change in Store.applyIncoming / hydrateResourceFromJson.

No screenshots, because nothing changes visually. The menu is just no longer stale.

🤖 Generated with Claude Code

useDriveApps published nothing until its whole mount chain had finished:
findSchema, the local app query, then an awaited getResource per app, one
after the other. Live events that landed meanwhile were held back too. After
a reload, an app whose sync delta could not apply to the page's copy
("incomplete Loro import ... missing base state") sits as a placeholder under
repair, and getResource waits up to 10 s on it, which is as long as the
money e2e waits for the "New app" menu item.

- Attach the listeners before anything else and fold every answer in as it
  arrives, reading the live resource each time, so no answer overrules a
  newer one and no slow read holds back the others.
- Read the query's members in parallel.
- Expose refresh(), and call it when "+ Add view" and a view tab's menu
  open, so opening a menu re-runs the query and re-reads what it knows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@michielbdejong michielbdejong added bug Something isn't working browser labels Sep 29, 2026
joepio added a commit that referenced this pull request Sep 30, 2026
# Conflicts:
#	browser/data-browser/src/chunks/AppPage/useDriveApps.live.test.tsx
#	browser/data-browser/src/chunks/TablePage/TableViewTabs.tsx
joepio added a commit that referenced this pull request Sep 30, 2026
…leViewTabs mock for useDriveApps' {apps, refresh} (#1904 x #1902), settle catalogs
@joepio

joepio commented Sep 30, 2026

Copy link
Copy Markdown
Member

Trekmeester: included in batch #1947 (run 36781515104; #1915 had been merged without it). It lands when that run is green; if this PR breaks something there, I take it out and say why here.

@joepio

joepio commented Oct 1, 2026

Copy link
Copy Markdown
Member

Trekmeester: included in batch #1966 (run 36898843808). It lands when that run is green; if this PR breaks something there, I take it out and say why here.

@joepio

joepio commented Oct 2, 2026

Copy link
Copy Markdown
Member

Landed through batch #1966 (squash 5e189e6, green run 36970563089). Fixes made while batching are listed in #1966.

@joepio joepio closed this Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

browser bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants