fix: enforce onComplete requirement in audio recorder overloads - #1082
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (11)
Included review availability: Your plan provides up to 8 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe transforming audio-recorder overloads in Angular, React, Solid, Svelte, and Vue now require ChangesAudio recorder typing
Estimated code review effort: 2 (Simple) | ~15 minutes Merge Risk: ⚪ Minimal · up to This localized change makes onComplete required for the transforming audio-recorder overload across supported frameworks and adds corresponding tests; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ai-angular/tests/inject-audio-recorder-types.test.ts`:
- Around line 1-8: Move the tests beside their related source modules and update
imports and Vitest discovery as needed:
packages/ai-angular/tests/inject-audio-recorder-types.test.ts (lines 1-8) beside
packages/ai-angular/src/inject-audio-recorder.ts;
packages/ai-react/tests/use-audio-recorder.test.tsx (lines 155-186) beside
packages/ai-react/src/use-audio-recorder.ts;
packages/ai-solid/tests/use-audio-recorder.test.ts (lines 125-141) beside
packages/ai-solid/src/use-audio-recorder.ts;
packages/ai-svelte/tests/create-audio-recorder.test.ts (lines 84-111) beside
packages/ai-svelte/src/create-audio-recorder.svelte.ts; and
packages/ai-vue/tests/use-audio-recorder.test.ts (lines 137-152) beside
packages/ai-vue/src/use-audio-recorder.ts. Preserve each test’s behavior and use
the adjacent *.test.ts naming convention.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 96cba639-9119-457f-ba91-cd306119069f
📒 Files selected for processing (11)
.changeset/soft-teams-work.mdpackages/ai-angular/src/inject-audio-recorder.tspackages/ai-angular/tests/inject-audio-recorder-types.test.tspackages/ai-react/src/use-audio-recorder.tspackages/ai-react/tests/use-audio-recorder.test.tsxpackages/ai-solid/src/use-audio-recorder.tspackages/ai-solid/tests/use-audio-recorder.test.tspackages/ai-svelte/src/create-audio-recorder.svelte.tspackages/ai-svelte/tests/create-audio-recorder.test.tspackages/ai-vue/src/use-audio-recorder.tspackages/ai-vue/tests/use-audio-recorder.test.ts
| /** | ||
| * Type-level tests for `injectAudioRecorder` overload selection (issue #1001). | ||
| * Scenario functions are never executed (the injectable requires an injection | ||
| * context at runtime), so `expectTypeOf` runs as a no-op and `tsc` | ||
| * (`test:types`) is what validates them. | ||
| */ | ||
| import { describe, expectTypeOf, it } from 'vitest' | ||
| import { injectAudioRecorder } from '../src/inject-audio-recorder' |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Co-locate the new type tests with their source modules.
These tests are in package-level tests/ directories. Move each test beside its related source module. Update relative imports and test discovery configuration if required.
packages/ai-angular/tests/inject-audio-recorder-types.test.ts#L1-L8: Move the test besidepackages/ai-angular/src/inject-audio-recorder.ts.packages/ai-react/tests/use-audio-recorder.test.tsx#L155-L186: Move the test besidepackages/ai-react/src/use-audio-recorder.ts.packages/ai-solid/tests/use-audio-recorder.test.ts#L125-L141: Move the test besidepackages/ai-solid/src/use-audio-recorder.ts.packages/ai-svelte/tests/create-audio-recorder.test.ts#L84-L111: Move the test besidepackages/ai-svelte/src/create-audio-recorder.svelte.ts.packages/ai-vue/tests/use-audio-recorder.test.ts#L137-L152: Move the test besidepackages/ai-vue/src/use-audio-recorder.ts.
As per coding guidelines, “Test files should be placed alongside source code as *.test.ts files using Vitest with happy-dom for DOM testing.”
📍 Affects 5 files
packages/ai-angular/tests/inject-audio-recorder-types.test.ts#L1-L8(this comment)packages/ai-react/tests/use-audio-recorder.test.tsx#L155-L186packages/ai-solid/tests/use-audio-recorder.test.ts#L125-L141packages/ai-svelte/tests/create-audio-recorder.test.ts#L84-L111packages/ai-vue/tests/use-audio-recorder.test.ts#L137-L152
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ai-angular/tests/inject-audio-recorder-types.test.ts` around lines 1
- 8, Move the tests beside their related source modules and update imports and
Vitest discovery as needed:
packages/ai-angular/tests/inject-audio-recorder-types.test.ts (lines 1-8) beside
packages/ai-angular/src/inject-audio-recorder.ts;
packages/ai-react/tests/use-audio-recorder.test.tsx (lines 155-186) beside
packages/ai-react/src/use-audio-recorder.ts;
packages/ai-solid/tests/use-audio-recorder.test.ts (lines 125-141) beside
packages/ai-solid/src/use-audio-recorder.ts;
packages/ai-svelte/tests/create-audio-recorder.test.ts (lines 84-111) beside
packages/ai-svelte/src/create-audio-recorder.svelte.ts; and
packages/ai-vue/tests/use-audio-recorder.test.ts (lines 137-152) beside
packages/ai-vue/src/use-audio-recorder.ts. Preserve each test’s behavior and use
the adjacent *.test.ts naming convention.
Source: Coding guidelines
|
Thanks for the PR, @harshlocham! 🙌 @tombeckenham will take a look. Automated pre-review checks
Automated triage — a human review follows. |
91a9599 to
987b9db
Compare
987b9db to
0cb6c33
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
View your CI Pipeline Execution ↗ for commit 4fdea07
☁️ Nx Cloud last updated this comment at |
343b75d to
6680705
Compare
Updated the audio recorder functions across multiple frameworks to require the `onComplete` option in their overloads. This change prevents type inference issues where unrelated options could lead to collapsing the `recording` and `stop()` types to `unknown`. Added type-level tests to ensure correct behavior and maintain the `AudioRecording` type when options do not include `onComplete`. Addresses issue TanStack#1001.
6680705 to
4fdea07
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
@tanstack/ai
@tanstack/ai-acp
@tanstack/ai-angular
@tanstack/ai-anthropic
@tanstack/ai-bedrock
@tanstack/ai-byteplus
@tanstack/ai-claude-code
@tanstack/ai-client
@tanstack/ai-code-mode
@tanstack/ai-code-mode-snippets
@tanstack/ai-codex
@tanstack/ai-cohere
@tanstack/ai-devtools-core
@tanstack/ai-durable-stream
@tanstack/ai-elevenlabs
@tanstack/ai-event-client
@tanstack/ai-fal
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-grok-build
@tanstack/ai-groq
@tanstack/ai-isolate-cloudflare
@tanstack/ai-isolate-daytona
@tanstack/ai-isolate-node
@tanstack/ai-isolate-quickjs
@tanstack/ai-isolate-quickjs-bun
@tanstack/ai-mcp
@tanstack/ai-memory
@tanstack/ai-mistral
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-opencode
@tanstack/ai-openrouter
@tanstack/ai-perplexity
@tanstack/ai-persistence
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-sandbox
@tanstack/ai-sandbox-cloudflare
@tanstack/ai-sandbox-daytona
@tanstack/ai-sandbox-docker
@tanstack/ai-sandbox-local-process
@tanstack/ai-sandbox-sprites
@tanstack/ai-sandbox-vercel
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-utils
@tanstack/ai-vercel-gateway
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/openai-base
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
Fixes #1001
🎯 Changes
The transforming useAudioRecorder overload could previously match options that did not provide onComplete. For example, passing only { onError } could select the transforming overload and cause recording / stop() to be inferred as unknown.
This updates the transforming overload across React, Vue, Solid, Svelte, and Angular so that onComplete is required when that overload is selected.
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
Bug Fixes
onCompletecallback is supplied.Tests