deps: retire legacy import + dead packages; put clipboard, device, glass-effect, and @expo/ui to real use - #152
Merged
Merged
Conversation
…torage The one-shot 'legacy-drafts-import' data migration read drafts out of the ORIGINAL Pulse app's AsyncStorage (recording_drafts key + media/ tree) and was the sole consumer of @react-native-async-storage/async-storage. The 2.0.0 replacement rollout (#86) is complete, so the import path is retired and the dependency removed. The data-migration runner stays: DATA_MIGRATIONS is now empty, with the retired task's id documented as never-reusable (installs that ran it still hold its dataMigration.legacy-drafts-import completion key). Part of #151.
Nothing imports it: RN's Metro setup never wired a global Buffer shim to it, and the upload pipeline reads bytes via expo-file-system, not Node Buffers. Part of #151.
The watch URL existed only behind the Watch button (open in browser) — there was no way to get the link itself out of the app to share into a chat or note (#69's missing-watch-link gap). The upload-complete alert now offers Copy link alongside Watch: it puts the same artifact URL on the clipboard via expo-clipboard's setStringAsync and confirms with the standard toast. Part of #151 (expo-clipboard was installed but unused).
The large model's picker note was a blanket platform ternary baked into
the catalog ('slow on Android'). Replace it with modelCaveat(model,
profile): a pure function fed by a DeviceProfile, adding a RAM floor —
under 4 GB the 574 MB q5_0 working set is memory-starved (swap/OOM), so
the picker now warns 'may be unstable on this device' on low-RAM phones
of either platform, and keeps 'slow on Android' (whisper.rn has no GPU
backend there) otherwise.
The expo-device read lives in device-profile.ts at the UI layer; models.ts
stays free of react-native imports so the pure-Node jest suite keeps
loading it (the EXPO_OS env trick is gone with the ternary). Part of #151
(expo-device was installed but unused).
The recorder's floating chrome (close button, control rail, lens chips) shared one flat rgba(0,0,0,0.35) scrim. A new GlassPill component renders those surfaces as native Liquid Glass (GlassView, dark-pinned so it stays legible over live camera video) when isLiquidGlassAvailable(), and falls back to the exact same scrim on Android and older iOS — no visual change there. Part of #151 (expo-glass-effect was installed but unused).
…m sheet via @expo/ui The model switcher was a hand-rolled RN Modal: slide animation, dim backdrop, tap-outside-to-close — no drag indicator, no swipe-to-dismiss, no detent physics. It now presents through @expo/ui's universal BottomSheet (SwiftUI sheet on iOS, Material 3 ModalBottomSheet on Android) with the RN content hosted inside via RNHostView, the documented RN-inside-native interop. The sheet surface is pinned to the app theme — presentationBackground (swift-ui modifier) on iOS, background (jetpack-compose modifier) on Android — because the app's manual light/dark override can diverge from the system scheme, and a system-schemed sheet behind app-themed content would go unreadable in the mismatch case. Component API (visible/onClose) is unchanged for both mount sites. Part of #151 (@expo/ui was installed but unused).
There was a problem hiding this comment.
Pull request overview
This PR cleans up dependency drift by removing truly-dead packages while wiring up previously-installed-but-unused Expo modules to improve UX (upload completion, device-aware model guidance, and more-native UI/visual chrome).
Changes:
- Remove legacy AsyncStorage draft migration codepath and drop unused deps (
@react-native-async-storage/async-storage,buffer). - Add device-aware transcription model caveats (
expo-device) while keeping the model catalog pure for Node/Jest. - Adopt native UI/visual components: on-device AI bottom sheet via
@expo/ui, and recorder chrome viaexpo-glass-effect; add “Copy link” on upload completion viaexpo-clipboard.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/features/transcription/models.ts | Moves platform/device warnings out of the catalog and introduces modelCaveat + device profile types/constants. |
| src/features/transcription/models.test.ts | Adds unit tests covering the new modelCaveat behavior. |
| src/features/transcription/model-switcher-modal.tsx | Replaces the RN Modal with a native bottom sheet and appends device-aware caveats in the model list. |
| src/features/transcription/device-profile.ts | Provides a UI-layer expo-device backed currentDeviceProfile() while keeping models.ts pure-Node. |
| src/features/recorder/lens-selector.tsx | Switches lens chip container to the new GlassPill chrome component. |
| src/features/recorder/close-button.tsx | Wraps the close icon in GlassPill for consistent recorder chrome styling. |
| src/features/recorder/camera-controls.tsx | Wraps camera control buttons in GlassPill (native glass where supported, scrim fallback otherwise). |
| src/db/migrate.tsx | Retires the legacy data migration task list (keeps runner, documents task id as non-reusable). |
| src/db/legacy-migration.ts | Removes the legacy AsyncStorage-based draft import implementation. |
| src/components/glass-pill.tsx | Introduces GlassPill (Liquid Glass on iOS 26+; scrim fallback elsewhere). |
| src/app/export.tsx | Adds “Copy link” on upload completion using expo-clipboard and toast confirmation. |
| package.json | Drops unused dependencies. |
| package-lock.json | Updates lockfile to reflect dependency removals. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…o-doctor 20/20 expo install --fix across the board: expo 57.0.8 -> 57.0.11 (+ 18 sibling expo packages), react-native 0.86.0 -> 0.86.2, reanimated 4.5.1, screens 4.26, worklets 0.10.1, eslint-config-expo 57.0.1. All in-range patch/minor moves per the SDK's own compatibility map - no majors here (those are tracked separately in #151). expo-doctor now passes 20/20: the version check is satisfied by the sweep, and react-native-background-actions joins the vetted reactNativeDirectoryCheck excludes - the directory flags it "Untested on New Architecture", but it is the deliberate, on-device-verified dataSync foreground-service wrapper for Android uploads (see keep-alive.ts for the rationale and the iOS exclusion). Native versions moved (react-native, reanimated, screens, worklets), so dev clients need a rebuild: npx expo run:ios -d / run:android. Part of #151.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/app/export.tsx:144
Clipboard.setStringAsync(watchUrl)is awaited via.then(...)but errors are not handled. IfsetStringAsyncrejects (e.g., platform/permission edge cases), this can surface as an unhandled promise rejection. Add a.catch(...)(or wrap in try/catch) to explicitly swallow/log failures while still acknowledging the alert.
void Clipboard.setStringAsync(watchUrl).then((ok) => {
if (ok) showToast('Link copied');
});
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
src/app/export.tsx:144
- Clipboard.setStringAsync() resolves to void (and rejects on failure); it does not return a boolean. As written,
okwill always be undefined so the toast never shows, and a rejection would be unhandled. Chain.then()/.catch()(or use async/await with try/catch) and show the toast on successful resolution.
// setStringAsync resolves true on success; failure (rare) still acknowledges so
// the alert never re-fires — the toast is only shown for a real copy.
void Clipboard.setStringAsync(watchUrl).then((ok) => {
if (ok) showToast('Link copied');
});
buffer is NOT unused - it is whisper.rn's Metro polyfill. whisper.rn
depends on safe-buffer, whose `require('buffer')` expects the Node
builtin; on React Native, Metro satisfies that bare specifier from the
npm buffer package, which only this app's direct dependency provides.
Removing it broke the bundle on-device:
The package at "node_modules/safe-buffer/index.js" attempted to import
the Node standard library module "buffer".
(safe-buffer <- whisper.rn <- transcription/whisper.ts)
The audit only checked first-party imports in src/; the lesson recorded
in #151: a dependency can be load-bearing purely as a transitive Node
polyfill, so removal candidates must also be checked against
node_modules consumers (grep for require('<pkg>') across installed
deps) or verified with a bundle pass, not just a source grep.
Part of #151.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/app/export.tsx:144
expo-clipboard'ssetStringAsyncresolvesvoid(it doesn't return a success boolean), sookwill beundefinedand the toast will never show. Use a.then(() => ...)and.catch(() => ...)(or try/await) to show the toast only on successful resolution.
// setStringAsync resolves true on success; failure (rare) still acknowledges so
// the alert never re-fires — the toast is only shown for a real copy.
void Clipboard.setStringAsync(watchUrl).then((ok) => {
if (ok) showToast('Link copied');
});
src/db/migrate.tsx:20
- The header comment says "never remove" shipped data-migration entries, but this PR removes the only shipped entry and leaves the list empty. Either keep a (no-op) placeholder entry to preserve the append-only invariant, or update the comment so it matches the new retirement approach.
* All one-shot data migrations, in execution order. APPEND new tasks at the end — never
* remove, rename, or reorder shipped entries (see data-migrations.ts for the task rules).
*
* Currently empty: the only shipped task was 'legacy-drafts-import' (drafts from the original
* Pulse app ≤ 1.2.x, read out of AsyncStorage), retired in 2.x after the replacement rollout
…NHostView imposes none On-device the sheet collapsed to one character per line: RNHostView's matchContents sizes the host to the RN content's intrinsic size, and RN flex content has no intrinsic width, so the layout solved to minimum width. Pin the content to the window width minus the native sheet's own 16pt-per-side padding, and drop the internal horizontal padding from 24 to 8 so the total 24pt visual gutter matches the old modal.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/app/export.tsx:144
Clipboard.setStringAsyncpromise rejection isn’t handled here. If the clipboard call throws/rejects (permissions/OS edge cases), this can surface as an unhandled promise rejection in RN. Consider adding a.catchso failures are swallowed (you already avoid showing the toast unless it returns true).
void Clipboard.setStringAsync(watchUrl).then((ok) => {
if (ok) showToast('Link copied');
});
src/features/transcription/model-switcher-modal.tsx:118
- The
modifiersselection falls back to the Android Jetpack Compose modifier for any non-iOS platform. That meansPlatform.OS === 'web'(and any future platforms) will still attempt to use the Android-only modifier path, which is very likely incorrect and can break non-Android builds. Consider explicitly checking forandroidand using an empty modifier list otherwise.
modifiers={
Platform.OS === 'ios'
? [presentationBackground(theme.background)]
: [background(theme.background)]
}>
… in light mode elsewhere The shared CloseButton got the Liquid Glass pill everywhere, but glass belongs only over live video: dark-pinned glass over a light themed background renders nearly transparent, leaving the white xmark invisible in light mode on the export screen and caption editor. New overVideo prop: the recorder keeps glass, every themed-screen call site keeps the original opaque scrim (visually identical to pre-branch).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/app/export.tsx:146
- Clipboard.setStringAsync(...) is awaited via .then() without a .catch(); if the promise rejects (e.g. clipboard unavailable/permission error), this becomes an unhandled promise rejection. Handle rejection explicitly so the app doesn’t log noisy errors in production/debug builds.
{
text: 'Copy link',
onPress: () => {
// setStringAsync resolves true on success; failure (rare) still acknowledges so
// the alert never re-fires — the toast is only shown for a real copy.
void Clipboard.setStringAsync(watchUrl).then((ok) => {
if (ok) showToast('Link copied');
});
acknowledgeDone();
},
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/features/transcription/model-switcher-modal.tsx:163
currentDeviceProfile()is called inside theMODELS.map(...)loop, so it runs once per model on every render. Since the profile values are static module constants, compute it once per render (e.g.,const profile = currentDeviceProfile();) and reuse it for allmodelCaveat(...)calls to keep the render path simpler and avoid repeated work if more models are added.
{MODELS.map((model) => {
const active = model.id === selectedId;
// Device-aware caveat (RAM floor / Android CPU-only inference) appended to the
// model's base note — computed here, not in the catalog, so models.ts stays pure.
const caveat = modelCaveat(model, currentDeviceProfile());
return (
…e App Store yet Reverts the retirement from earlier in this branch (a1febfe). The retirement assumed the 2.0.0 replacement rollout was complete, but 2.0.0 is still unpublished (App Review, #147) - every original Pulse user on the App Store is still on <=1.2.x, so the FIRST public 2.x release must carry the import or those users' drafts are stranded. legacy-migration.ts and the DATA_MIGRATIONS wiring come back exactly as shipped; @react-native-async-storage/async-storage returns at the SDK 57 expected version (2.2.0). Retire again only after 2.x has been live on the App Store long enough for the upgrade wave to pass - tracked in Fixed in the retirement plan noted on #151.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/features/transcription/model-switcher-modal.tsx:162
- Now that the device profile is cached once per render, reuse that cached value here so the caveat computation is stable and avoids repeating the same device lookup for every row.
// Device-aware caveat (RAM floor / Android CPU-only inference) appended to the
// model's base note — computed here, not in the catalog, so models.ts stays pure.
const caveat = modelCaveat(model, currentDeviceProfile());
src/features/transcription/model-switcher-modal.tsx:76
currentDeviceProfile()returns static device constants (Platform.OS / Device.totalMemory). Calling it inside the model list loop recomputes the same value once per model render. Cache it once per render and reuse it for each row.
This issue also appears on line 160 of the same file.
const { width: windowWidth } = useWindowDimensions();
const sheetWidth = windowWidth - 32;
const { data } = useLiveQuery(selectedModelQuery, []);
const selectedId = data[0]?.value ?? null;
const status = useTranscriptionStatus();
The window-derived width assumed the sheet spans the window - true on portrait iPhone, false on tablets: iPad presents SwiftUI sheets as narrower centered page/form sheets (~540pt+) and M3 caps ModalBottomSheet at 640dp, so window-pinned content would overflow the sheet edge (supportsTablet is true). Cap at 500pt: fits the narrowest tablet presentation, and no phone window reaches the cap so iPhone rendering is unchanged.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
src/features/recorder/use-recorder.ts:415
- This call site still uses
launchImageLibraryAsync/UIImagePickerPreferredAssetRepresentationModedirectly. After switching to a namespace import, gate the iOS-only option behindPlatform.OS === 'ios'and only include it when the enum exists; this avoids crashes on Android/web while keeping the iOS behavior.
mediaTypes: ['videos'],
// Ask PHPicker for the ORIGINAL representation (no transcode) — the documented
// replacement for the deprecated `videoExportPreset: Passthrough`. Our own
// decideImport owns normalization, so the picker must not re-encode first.
preferredAssetRepresentationMode: UIImagePickerPreferredAssetRepresentationMode.Current,
src/features/transcription/model-switcher-modal.tsx:121
- The sheet surface is theme-pinned via native modifiers on iOS/Android, but on platforms where
sheetBackgroundModifiers()returns[](tests/web persheet-background.ts), this container no longer setsbackgroundColor, so the sheet can render with a transparent/default surface behind themed content. KeepbackgroundColor: theme.backgroundon the hosted RN root as a cross-platform fallback.
<View
style={[
styles.sheet,
{ width: sheetWidth, paddingBottom: insets.bottom + Spacing.three },
]}>
src/features/transcription/model-switcher-modal.tsx:76
useWindowDimensions()can report a very small width (or 0 in some test/web layouts).windowWidth - 32can go negative, which produces an invalid negativewidthstyle and can cause layout warnings/broken rendering. Clamp the computed width to a minimum of 0.
This issue also appears on line 117 of the same file.
const { width: windowWidth } = useWindowDimensions();
const sheetWidth = Math.min(windowWidth - 32, 500);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/features/transcription/model-switcher-modal.tsx:76
sheetWidthcan become negative whenwindowWidth < 32(e.g., very narrow web viewport, initial 0-width during layout, or certain test environments). React Native styles treat negative widths as invalid and this can lead to layout warnings or collapsed content. Clamp the computed width to a non-negative value before applying it.
const { width: windowWidth } = useWindowDimensions();
const sheetWidth = Math.min(windowWidth - 32, 500);
…luminance adaptation flipped the pill light/dark as the camera panned Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/features/transcription/model-switcher-modal.tsx:76
sheetWidthcan become negative whenuseWindowDimensions()reports a very small/zero width (common during initial layout on web/SSR or some test environments). A negativewidthstyle can cause layout warnings or unexpected rendering in RN. Clamp the computed width to a minimum of 0 (or 1) before applying it.
const { width: windowWidth } = useWindowDimensions();
const sheetWidth = Math.min(windowWidth - 32, 500);
…creenModal; dark-mode sheet elevation - Toast: iOS renders inside FullWindowOverlay (its own UIWindow) — a root-level sibling paints behind fullScreenModal presentations, so export's 'Link copied' was firing invisibly. Device-verified over the export modal. - Upload-complete: Alert.alert -> themed modal. An alert's Cancel row renders identically to the real actions and read as a third action; dismissal is now an explicit ✕ (plus backdrop tap / hardware back), leaving Copy link and Watch as the only visually-weighted choices. Hairline theme.border outline + shadow (the action-menu card treatment) so the dark-mode card separates from the dimmed backdrop. Segmented confirmation stays a plain native alert. - Icon: add Android Lucide mapping for 'link' (used by the Copy link button). - On-device AI sheet: dark mode pins the surface to the ELEVATED secondary background instead of pure black (zero separation over a dimmed black screen), mirroring iOS elevated dark modals / M3 tonal elevation; on-sheet elements step up one level so they don't blend. Light mode unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/features/transcription/model-switcher-modal.tsx:86
sheetWidthcan become negative (e.g. very small windows or transient 0-width during orientation changes). A negativewidthstyle can cause layout warnings or collapsed rendering. Clamp the computed width to a minimum of 0 (or 1) before applying the 500pt cap.
const sheetWidth = Math.min(windowWidth - 32, 500);
morepriyam
added a commit
that referenced
this pull request
Aug 12, 2026
deps: retire legacy import + dead packages; put clipboard, device, glass-effect, and @expo/ui to real use
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 1 of #151: retire dead dependencies, align every package to the SDK 57 expected versions, put the four installed-but-never-imported packages to real use, and fix what device testing surfaced. Follow-up work (React Compiler lint, whisper.rn 0.7, majors, formatting PR, remaining removal candidates) is scoped in the issue: #151 (comment)
Removals — both reverted within this branch (kept for the record)
@react-native-async-storage/async-storage+ the legacy ≤1.2.x draft importbufferrequire('buffer')). Removal broke the on-device bundle; the "check node_modules consumers or do a bundle pass" rule is now recorded in deps: prune dead packages, take pending updates, align usage with current official APIs #151.Net effect: no packages removed in this PR — the removal attempts hardened the audit rules instead.
Version alignment — expo-doctor 20/20
npx expo install --fixacross the board: expo 57.0.11 (+18 sibling packages), react-native 0.86.2, reanimated 4.5.1, screens 4.26, worklets 0.10.1, eslint-config-expo 57.0.1 — all in-range per the SDK 57 compatibility map. Plus the two non-SDK in-range bumps: lucide-react-native 1.30, @types/react 19.2.18.react-native-background-actionsjoined the vetted doctor excludes (deliberate, device-verified dataSync foreground service — see keep-alive.ts). Native versions moved: dev clients need a rebuild.Adoptions (installed but unused → wired per official docs)
modelCaveat(model, profile)adds a 4 GB RAM floor ("may be unstable on this device" for the 574 MB large model, both platforms) alongside "slow on Android" (whisper.rn is CPU-only there). expo-device read isolated indevice-profile.ts;models.tsstays pure-Node for jest (+4 tests). The oldEXPO_OSternary in the catalog is gone.GlassPill: the recorder chrome (close button, control rail, lens chips) renders native Liquid Glass on iOS 26+ (dark-pinned for legibility over live video), pixel-identical scrim fallback elsewhere.RNHostView, surface pinned to the app theme (presentationBackground/backgroundmodifiers) so a manual light/dark override can't mismatch.API currency
videoExportPreset: Passthrough→ documentedpreferredAssetRepresentationMode: Current(PHPicker hands over original bytes untranscoded;decideImportkeeps owning normalization)./legacyimports (media-library save, file-system md5) are deliberate, documented exceptions.Fixes from on-device testing (iPhone, iOS 26)
RNHostView matchContents+ RN flex content (no intrinsic width) rendered one character per line; content now pinned to window width minus the sheet's built-in padding. Verified fixed on device.overVideoprop: recorder keeps glass, themed screens keep the original opaque scrim.fullScreenModal(a real UIKit presentation above the RN root), so the root-level toast painted behind it on iOS. The toast now renders insideFullWindowOverlay(react-native-screens) — its own UIWindow above every presentation. Android unchanged.theme.borderoutline + shadow so it separates from the dimmed backdrop in dark mode. Segmented confirmation stays a native alert. (linkgained its Android Lucide mapping.)Kept despite zero imports (documented)
expo-system-ui(userInterfaceStyleon Android),expo-constants/expo-linking/expo-status-bar(expo-router companions),react-native-nitro-image(video-trim fork peer), and the config-plugin-only expo packages.Verification
tsc --noEmitclean, eslint clean, jest 126 passed (+4 new),expo-doctor20/20,expo install --checkcleanpreferredAssetRepresentationMode: Current✓ · large-download alert presents above the native sheet ✓ · prompt modal in light mode ✓linkglyph)Part of #151.