Mobile: Fix #14804: Migrate expo-av to expo-audio#14847
Open
gherardi wants to merge 11 commits intolaurent22:devfrom
Open
Mobile: Fix #14804: Migrate expo-av to expo-audio#14847gherardi wants to merge 11 commits intolaurent22:devfrom
gherardi wants to merge 11 commits intolaurent22:devfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughMigrated the audio recording functionality from the deprecated Changes
Suggested reviewers
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/app-mobile/components/voiceTyping/AudioRecordingBanner.tsx (1)
108-108: Consider memoising recording options.
recordingOptions()is called on every render, creating a new object each time. IfuseExpoAudioRecordercompares options by reference, this could cause unnecessary re-initialisations. Consider usinguseMemo:♻️ Suggested refactor
+const options = useMemo(() => recordingOptions(), []); -const recorder = useExpoAudioRecorder(recordingOptions()); +const recorder = useExpoAudioRecorder(options);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app-mobile/components/voiceTyping/AudioRecordingBanner.tsx` at line 108, The recordingOptions() call is creating a new object each render and may trigger unnecessary re-initialisations in useExpoAudioRecorder; wrap the options in useMemo (e.g., const memoRecordingOptions = useMemo(() => recordingOptions(), [/* any deps like language/sampleRate */])) and pass memoRecordingOptions to useExpoAudioRecorder so the reference is stable; update AudioRecordingBanner.tsx to import useMemo if needed and ensure the dependency array includes any values recordingOptions depends on.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app-mobile/components/voiceTyping/AudioRecordingBanner.tsx`:
- Around line 139-151: The recorder can be left prepared if an exception happens
after prepareToRecordAsync() but before isRecordingRef.current is set; to fix,
set isRecordingRef.current = true immediately after prepareToRecordAsync()
(before recorder.record()) or add explicit cleanup in the catch block that
checks the recorder's prepared state and calls
recorder.stop()/recorder.unloadAsync() before clearing isRecordingRef; update
the code paths that setRecordingState(RecorderState.Error) and setError(...)
accordingly so the prepared recorder is always stopped/unloaded (reference
prepareToRecordAsync, isRecordingRef, recorder.record, recorder.stop,
setRecordingState, setError).
---
Nitpick comments:
In `@packages/app-mobile/components/voiceTyping/AudioRecordingBanner.tsx`:
- Line 108: The recordingOptions() call is creating a new object each render and
may trigger unnecessary re-initialisations in useExpoAudioRecorder; wrap the
options in useMemo (e.g., const memoRecordingOptions = useMemo(() =>
recordingOptions(), [/* any deps like language/sampleRate */])) and pass
memoRecordingOptions to useExpoAudioRecorder so the reference is stable; update
AudioRecordingBanner.tsx to import useMemo if needed and ensure the dependency
array includes any values recordingOptions depends on.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 08beb34b-eed4-4769-8560-527f4c6ad179
⛔ Files ignored due to path filters (2)
packages/app-mobile/ios/Podfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
packages/app-mobile/components/voiceTyping/AudioRecordingBanner.tsxpackages/app-mobile/jest.setup.jspackages/app-mobile/package.jsonrenovate.json5
💤 Files with no reviewable changes (1)
- packages/app-mobile/jest.setup.js
gherardi
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/app-mobile/components/voiceTyping/AudioRecordingBanner.tsx (1)
38-45: Remove linearPCM options unnecessary for MPEG4AAC encoding.The
linearPCMBitDepth,linearPCMIsBigEndian, andlinearPCMIsFloatproperties are documented as specific to LINEARPCM output format, not MPEG4AAC. These should be removed from the iOS recording options to avoid confusion and improve code clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app-mobile/components/voiceTyping/AudioRecordingBanner.tsx` around lines 38 - 45, In AudioRecordingBanner.tsx update the ios recording options object used for iOS (the ios constant passed to the recorder) by removing the LINEARPCM-specific properties linearPCMBitDepth, linearPCMIsBigEndian, and linearPCMIsFloat since the outputFormat is IOSOutputFormat.MPEG4AAC; leave extension, audioQuality, and outputFormat as-is so the iOS options only include settings relevant to MPEG4AAC.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app-mobile/components/voiceTyping/AudioRecordingBanner.tsx`:
- Around line 95-102: The resetAudioMode function calls setAudioModeAsync with
an invalid AudioMode property; remove the non-existent allowsBackgroundRecording
key from the object passed to setAudioModeAsync (leave allowsRecording,
playsInSilentMode, shouldPlayInBackground and any other valid AudioMode props
intact) so resetAudioMode and its call to setAudioModeAsync use only supported
expo-audio AudioMode parameters.
---
Nitpick comments:
In `@packages/app-mobile/components/voiceTyping/AudioRecordingBanner.tsx`:
- Around line 38-45: In AudioRecordingBanner.tsx update the ios recording
options object used for iOS (the ios constant passed to the recorder) by
removing the LINEARPCM-specific properties linearPCMBitDepth,
linearPCMIsBigEndian, and linearPCMIsFloat since the outputFormat is
IOSOutputFormat.MPEG4AAC; leave extension, audioQuality, and outputFormat as-is
so the iOS options only include settings relevant to MPEG4AAC.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d1cafecd-78c0-4a44-92e3-eb4fe8cc1534
📒 Files selected for processing (1)
packages/app-mobile/components/voiceTyping/AudioRecordingBanner.tsx
Collaborator
|
Thank you for working on this!
The It may be necessary to update |
Collaborator
|
Note: The code changes look good to me! However, I plan to test this locally on web and Android before marking this as ready for review. |
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
2 participants
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.
Fixes: #14804
Description:
This pull request migrates mobile voice recording from the deprecated
expo-avpackage toexpo-audio.The main problem addressed is that the app was still relying on
expo-avAPIs for audio recording even though Expo has deprecated that package in favor ofexpo-audio. During the migration, a few related issues also had to be handled to preserve existing behavior and stability:Changes:
useAudioRecorder()anduseAudioRecorderState(). This was necessary becauseexpo-audiomanages recording through hooks rather thannew Audio.Recording(). It also lets the component read the duration directly from recorder state instead of manually subscribing to status updates.getRecordingPermissionsAsync()andrequestRecordingPermissionsAsync(). This replaces the oldexpo-avpermission API and keeps the current behavior of checking permissions before starting a recording.recordingOptions()to match theexpo-audioshapemsleep(500)workaround after a new permission grant. This was retained intentionally to avoid the known iOS race condition where the first recording attempt can fail immediately after the permission dialog. (expo-av Recording Error: This experience is currently in the background, so the audio session could not be activated. expo/expo#21782)recorder.uriand passed intorecordingToSaveDatafunction. This matches theexpo-audioAPI and removes the previous dependence on the old recording object methodsAI disclosure:
This Pull Request was developed with the assistance of a local LLM. The AI helped clarify the
expo-audiodocumentation, troubleshoot specific bugs encountered during implementation, and refine the final code for better optimization and performance.Testing:
All tests in
packages/app-mobilepassed successfully. I’ve also attached a visual test for referenceRegistrazione.schermo.2026-03-20.alle.14.47.18.mov