move sonora to lbry apps

[zeeghazi]

Summary

• Move Sonora to Lbry apps.
• Add Sonora logo.
• fix routes and remove Sonora HomePage

[github-actions]

Code Review: Move Sonora to LBRY Apps

Overview

This PR moves the Sonora audio app into the LBRY apps section, adds a proper SVG logo, wires up TanStack Router file-based routes, and introduces six new page components (Browse, Record, Upload, Archive/Collection, Studio, Market). The scope is reasonable and the overall structure follows the patterns already established by Emporium.

---

Bugs / Correctness Issues

1. Memory leaks in RecordPage.tsx and UploadPage.tsx

URL.createObjectURL() is called but URL.revokeObjectURL() is never called. The object URL persists for the entire lifetime of the document unless explicitly revoked. Add cleanup:

// RecordPage.tsx — in stopRecording / discard handler
if (recordedUrl) URL.revokeObjectURL(recordedUrl);

Similarly, when a new file is selected in UploadPage.tsx, the old URL should be revoked before creating a new one.

2. MediaStream not cleaned up on unmount in RecordPage.tsx

If the user navigates away while recording, streamRef.current is never released, keeping the microphone active (browser mic indicator stays on). Add a cleanup effect:

useEffect(() => {
  return () => {
    streamRef.current?.getTracks().forEach(t => t.stop());
    if (timerRef.current) clearInterval(timerRef.current);
  };
}, []);

3. $principal dynamic routes render the same component as the parent and ignore the param

archive.$principal.lazy.tsx and studio.$principal.lazy.tsx both import and render the same ArchivePage/StudioPage as their non-parameterised siblings. The param is never read inside those pages — they both get user state from useAppSelector. Either:

• Consume useParams() to load a different user's NFTs (the apparent intent — e.g. for deep linking to another user's collection), or
• Remove the $principal routes entirely until that feature is ready.

---

Code Quality

4. alert() in UploadPage.tsx

alert("Please select an audio file");

Using window.alert breaks the design system's UX pattern. The rest of the app uses toast/inline error messages — please use the same approach here.

5. convertToAudio defined inside BrowsePage on every render

This pure helper doesn't close over any component state:

// Currently inside BrowsePage — recreated on every render
const convertToAudio = (arweaveAudio: ArweaveAudio): Audio => { ... }

Move it outside the component (or to a shared utils file) so it isn't recreated each render.

6. Stale/missing dependency in MarketPage.tsx

useEffect(() => {
  refreshMarketAudioNFTs(1, 8, false);
}, []); // ← refreshMarketAudioNFTs missing

ESLint's exhaustive-deps rule will flag this. Wrap refreshMarketAudioNFTs in useCallback in the hook (or add it to the dep array) to avoid potential stale-closure bugs.

7. Navigation inconsistency in SonoraLayout.tsx

The tab bar exposes four routes: Browse, Record, Collection, Shop. But two additional routes (/upload and /studio) are registered in the router with no nav entry. They're only reachable via sidebar links buried inside other pages. If Upload and Studio are intentional secondary flows, consider whether they should be in the tab bar or stay as unlinkable routes; if they're scaffolding for future work, a // TODO comment or a feature-flag guard would make the intent clear.

8. JSX indentation inconsistency

ArchivePage.tsx, MarketPage.tsx, and StudioPage.tsx have their <Helmet> and the outer grid <div> indented at the root level of the file (not indented under the fragment <>), while BrowsePage.tsx correctly indents everything. Minor but worth making consistent.

---

Performance

9. audios.length in BrowsePage useEffect deps

useEffect(() => {
  if (audios.length === 0 && !loading && !error) {
    dispatch(fetchAudios({ reset: true }));
  }
}, [dispatch, audios.length, loading, error]);

This re-evaluates whenever audios.length changes (i.e., after every "load more"). A useRef initialised flag is cleaner:

const initialised = useRef(false);
useEffect(() => {
  if (!initialised.current) {
    initialised.current = true;
    dispatch(fetchAudios({ reset: true }));
  }
}, [dispatch]);

---

What's Good

• The Sonora SVG logo is clean and well-structured with named gradient IDs (avoids global ID collisions).
• Removing the comingSoon: true flag and the picsum.photos placeholder URL is exactly the right change.
• Lazy-loading all Sonora routes follows the existing Emporium/Perpetua pattern and avoids bundle bloat.
• Helmet metadata on every page is a good SEO/accessibility practice.
• Error and empty-state handling in all pages is solid.
• The _auth route parent correctly enforces authentication for all Sonora sub-routes.

---

Summary

The main blockers are the memory leak / missing stream cleanup in RecordPage (items 1–2) and the unimplemented $principal routes that silently render the wrong content (item 3). The rest are quality improvements that should be straightforward to address.

[github-actions]

Code Review — PR #143: Move Sonora to LBRY Apps

Overview

This PR migrates Sonora out of the core app into the LBRY apps structure, adds a proper layout with navigation tabs, introduces five new pages (Browse, Archive, Market, Record, Studio, Upload), adds owner profile links from AudioCards, and swaps the placeholder Picsum logo for a real SVG. The structure mirrors the existing Emporium pattern and is generally clean.

---

Potential Bugs

1. Likely infinite re-render in ArchivePage

useStudioAudioNFTs and useMarketAudioNFTs were both updated to wrap their refresh functions in useCallback, but useUserAudioNFTs (used by ArchivePage) was not updated. If that hook returns an unstable function reference, this effect will loop:

// ArchivePage.tsx
useEffect(() => {
  if (targetPrincipal) {
    refreshAudioNFTs(targetPrincipal, 1, false);
  }
}, [targetPrincipal, refreshAudioNFTs]); // refreshAudioNFTs re-created every render → infinite loop

useUserAudioNFTs should receive the same useCallback treatment as the other two hooks.

2. MediaRecorder compatibility — Safari/iOS will silently fail

// RecordPage.tsx
const mediaRecorder = new MediaRecorder(stream);
// ...
const blob = new Blob(chunks, { type: "audio/webm" });

audio/webm is not supported in Safari or iOS WebKit. MediaRecorder will either throw on construction or produce an unusable blob. Use MediaRecorder.isTypeSupported() to pick a supported MIME type:

const mimeType = ['audio/webm', 'audio/ogg', 'audio/mp4'].find(t => MediaRecorder.isTypeSupported(t)) ?? '';
const mediaRecorder = new MediaRecorder(stream, mimeType ? { mimeType } : {});

3. Pagination condition inconsistency

ArchivePage uses pagination.hasMore for both the guard and the "Load More" button visibility, while MarketPage and StudioPage use pagination.page < pagination.totalPages. If the underlying pagination state shape differs across hooks (e.g. useUserAudioNFTs provides hasMore, others provide totalPages), one of these will always be wrong. They should use a consistent property, or a single convention should be established across all paginated hooks.

---

Code Quality

4. Mutation of converted object in BrowsePage

const audioItem = convertToAudio(arweaveAudio);
audioItem.size = formatFileSize(audioItem.size); // mutates immediately after construction

This is fragile. Either convertToAudio should accept a formatter, or formatFileSize should be called inside convertToAudio. The separate mutation step also means the size field in the Audio type is sometimes a raw bytes string and sometimes a formatted display string, which is a type-system lie.

5. Drag-and-drop flickering in UploadPage

const handleDragLeave = (e: React.DragEvent) => {
  e.preventDefault();
  setIsDragging(false); // fires when dragging over a child element
};

dragleave fires when the pointer moves into a child element, causing the border to flash off and back on. Fix with a relatedTarget check:

const handleDragLeave = (e: React.DragEvent) => {
  e.preventDefault();
  if (!e.currentTarget.contains(e.relatedTarget as Node)) {
    setIsDragging(false);
  }
};

6. BrowsePage initial-fetch guard is fragile

useEffect(() => {
  if (audios.length === 0 && !loading && !error) {
    dispatch(fetchAudios({ reset: true }));
  }
}, [dispatch, audios.length, loading, error]);

If the user genuinely has zero results (no audio on Arweave), this will re-dispatch every time the component mounts. A dedicated initialized flag in the slice (as Emporium does) is more reliable than inferring from length.

---

Minor / Style

7. Hardcoded gateway URL

const arweaveUrl = `https://arweave.net/${transactionId}`;

Appears in both RecordPage and UploadPage. If this pattern exists elsewhere in the codebase it is consistent, but consider extracting it to a shared constant so it is easy to swap gateways.

8. Upload route not in navigation tabs

SonoraLayout tabs are: Browse · Record · Archive · Studio · Market. The Upload page exists as a route (/app/sonora/upload) but is only reachable via a small link inside the Record page. If this is intentional (upload is a sub-action of Record) it should be documented; otherwise it may confuse users who want to upload without recording.

9. SonoraLayout — spread for activeOptions is verbose

{...("exact" in tab && tab.exact ? { activeOptions: { exact: true } } : {})}

Since only the Browse tab has exact, this can be simplified to a direct conditional on the index or a exact?: boolean field:

activeOptions={tab.exact ? { exact: true } : undefined}

---

Security

No concerns identified. Principal values travel through TanStack Router's typed params (no raw interpolation). The shortenPrincipal utility is used consistently for display. e.stopPropagation() on owner links is correctly applied to prevent unintended card-selection side effects.

---

Summary

Severity	Count
Bug (likely infinite re-render)	1
Bug (browser compatibility)	1
Bug (possible incorrect pagination)	1
Code quality	3
Minor/style	3

The overall approach is solid and consistent with the existing Emporium app pattern. The three bugs above are worth fixing before merge, especially the potential infinite loop in ArchivePage and the Safari recording issue given that Alexandria targets a wide audience.