Skip to content

fix(mux-input): keep the videos browser's grid while a hidden tool's query reconnects - #2179

Draft
stipsan wants to merge 1 commit into
mainfrom
cody/mux-input-keep-videos-browser-on-reveal-8079
Draft

stipsan wants to merge 1 commit into
mainfrom
cody/mux-input-keep-videos-browser-on-reveal-8079

Conversation

@stipsan

@stipsan stipsan commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

Description

With beta.reactActivityMode, returning to the Videos tool replaced the whole grid with a spinner for ~700ms, then remounted every video card (1648 components) and refetched all 14 animated thumbnails. The Studio keeps recently used tools mounted behind <Activity>, which tears down their subscriptions on hide and re-subscribes on reveal. useAssets read its listenQuery through createHookFromObservableFactory, whose observable starts every subscription with a loading tuple ([undefined, true]), so on reveal assets went empty and isLoading true until the listener's first result arrived.

useAssets now subscribes to the memoized listenQuery observable with react-rx's useObservable directly. Its store keeps the last emission across re-subscriptions, so the grid stays on screen while the query reconnects in the background, and only a query that has never emitted (the first load, a new search or sort) is loading. The asset source's InputBrowser shares the hook, so it gets the same behaviour. That retention is react-rx 7 behaviour rather than a documented guarantee (the per-component store entry keeps its snapshot across the share reset), so the hook test pins it and the code comment points there for the next react-rx bump.

What to review

  • src/hooks/useAssets.ts: useMemo of the observable plus useObservable(assetDocuments$, undefined); isLoading is "no emission yet". The returned shape is unchanged.
  • src/hooks/useAssets.test.tsx: the hook test. The @testing-library/react and jsdom devDependencies it needs are already on main since fix(mux-input): mount the Mux player only after clicking the poster #2074, so this PR no longer touches package.json or the lockfile.

Affects sanity-plugin-mux-input.

Testing

  • src/hooks/useAssets.test.tsx (jsdom, sanity fully mocked with useDocumentStore and a collate for published documents, so the test does not load the Studio package): the loaded videos stay rendered across an <Activity> hide/show cycle while the query re-subscribes once, and the next emission updates them. On main the component shows loading after the reveal.
  • pnpm --filter sanity-plugin-mux-input exec vitest run, pnpm build, pnpm lint, pnpm knip, pnpm format are clean.
  • Measured in dev/test-studio (/home/mux, reactActivityMode on) with a headed Playwright harness that switches to Vision, waits 7s, switches back and records React commits, mounted components, requests and painted frames (CDP screencast) until the page is quiet for 30 frames. Before: spinner mounted at 49ms, the grid gone until +766ms, 1835 components mounted, 14 thumbnail requests to image.mux.com. After: the grid is on screen from the first painted frame, 0 components mounted, 2 thumbnail requests. The 3 listen + 3 query reconnects remain (they are the Studio stores' cold start on reveal, and the data they deliver is identical). Frame counts are not comparable for this tool because the thumbnails are animated GIFs.

Videos tool before: spinner then full remount
Videos tool after: grid stays on screen

Notes for release

Fixes the Videos tool (and the Mux asset source) replacing its grid with a spinner and reloading every thumbnail when returning to it in a Studio with beta.reactActivityMode enabled.

To show artifacts inline, enable in settings.

Open in Web Open in Cursor 

@vercel

vercel Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
plugins-studio Ready Ready Preview Oct 29, 2026 4:08pm UTC
1 Skipped Deployment
Project Deployment Actions Updated
plugins-e2e-test-studio Ignored Ignored Oct 29, 2026 4:08pm UTC

Request Review

@stipsan stipsan added the ✨ ai PR authored by an AI agent (🤖 bot is for CI automation/Renovate) label Oct 13, 2026 — with Cursor
@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

✅ E2E Tests

🟢 46 passed • view full report • view run

Studio: https://plugins-e2e-test-studio-7bhs94zuw.sanity.dev

Datasets: pr-2179-chromium-40908382093, pr-2179-firefox-40908382093

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 39.08% 7936 / 20304
🔵 Statements 39.03% 8534 / 21863
🔵 Functions 34.67% 2171 / 6261
🔵 Branches 31.18% 4615 / 14797
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
plugins/sanity-plugin-mux-input/src/hooks/useAssets.ts 100% 100% 100% 100%
Generated in workflow #11464 for commit c24509d by the Vitest Coverage Report Action

@cursor

cursor Bot commented Oct 13, 2026

Copy link
Copy Markdown

Review: approve

Verified

  • useAssets.test.tsx passes on this branch. With main's useAssets.ts under the same test it fails with expected 'loading' to be 'a,b' after the reveal, as described.
  • Root cause holds in sanity 6.19: createHookFromObservableFactory builds concat(of({type: 'loading'}), …) with a scan seeded by the loading tuple, so every re-subscription necessarily restarts at [undefined, true]. No way to keep the grid without leaving that hook.
  • In react-rx 7.0.1 the per-component store entry keeps the last snapshot across the share reset (see inline note), and the 10 ms wait in the test is what makes the source release and subscriptions reach 2, so the test exercises the real reconnect path.
  • documentStore.listenQuery only composes operators (no subscription until subscribed), so building it in useMemo during render is side-effect free. Errors still surface through render (getSnapshot rethrows), same as before. A changed search or sort gets a new observable and therefore a loading state, same as before.
  • react-rx and rxjs are already direct dependencies, useAssets is not exported from _exports, CI green, lockfile diff is the two importer entries.

Non-blocking

  • vi.mock('sanity', importOriginal) loads the whole sanity package into jsdom (~2.3 s import here) for collate alone. A full mock with a two-line collate would make this test near-instant. Optional.

Verdict

Approve. Small diff, returned shape unchanged, the measurement claims that can be checked in code hold, and the test fails on main.

@cursor
cursor Bot force-pushed the cody/mux-input-keep-videos-browser-on-reveal-8079 branch from a0f4450 to 99ea258 Compare October 13, 2026 15:19
@changeset-bot

changeset-bot Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: c24509d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
sanity-plugin-mux-input Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@cursor

cursor Bot commented Oct 13, 2026

Copy link
Copy Markdown

Re-review (99ea258): approve

Both non-blocking points are addressed; nothing new to raise.

Verified

  • useAssets.test.tsx passes on this head, and the full sanity mock takes the file's import time from ~2.3 s to ~0.2 s. The two-line collate stand-in is only ever fed published documents, which is what the comment says and what the test does.
  • The code comment now states that the retention is react-rx behaviour rather than a documented guarantee and points at the test for the next react-rx bump; the description says the same.
  • One informational note, not an issue: with sanity fully mocked the test file can no longer be run against main's useAssets.ts as-is (that version imports createHookFromObservableFactory, which the mock does not provide). The "on main it shows loading" claim was verified with the previous importOriginal version of the same assertions in the first review, and the hook change itself is unchanged since then.
  • CI green, lockfile unchanged from the first review, useAssets still internal.

Verdict

Approve. The non-blocking thread is addressed and can be resolved on your side.

…query reconnects

With `beta.reactActivityMode` the Studio keeps recently used tools mounted behind
`<Activity>`, which tears down their subscriptions on hide and re-subscribes on
reveal. `useAssets` read its query through `createHookFromObservableFactory`, whose
observable starts every subscription with a loading tuple, so returning to the Videos
tool replaced the whole grid with a spinner and remounted it (and refetched every
thumbnail) once the listener's first result arrived.

Subscribe to the memoized `listenQuery` observable with react-rx's `useObservable`
directly: it keeps the last emission across re-subscriptions, so the grid stays put
while the query reconnects, and only a query that has never emitted (first load, a
new search or sort) is loading.

Co-authored-by: Cody Olsen <stipsan@users.noreply.github.com>

This branch was successfully deployed

1 active deployment
Preview – plugins-studio — c24509d2 Deployed Oct 29, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✨ ai PR authored by an AI agent (🤖 bot is for CI automation/Renovate)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants