Add typeguards for frame processors in order to avoid dual package hazard#608
Add typeguards for frame processors in order to avoid dual package hazard#608
Conversation
🦋 Changeset detectedLatest commit: 3209a24 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
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 |
…sdks into lukas/processor-typeguards
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
🐛 1 issue in files not directly in the diff
🐛 New type guards and symbols not exported from package public API (packages/livekit-rtc/src/index.ts:53-57)
The new type guard functions (isFrameProcessor, isAudioFrameProcessor, isVideoFrameProcessor), FrameProcessorSymbol, and FrameProcessorType are defined in frame_processor.ts but not re-exported from index.ts, which is the package's public entry point.
Root Cause and Impact
The entire purpose of this PR is to provide type guards to avoid the dual package hazard with instanceof checks. However, consumers of the @livekit/rtc-node package cannot access these new utilities because they are not exported from packages/livekit-rtc/src/index.ts:53-57:
export {
FrameProcessor,
type FrameProcessorStreamInfo,
type FrameProcessorCredentials,
} from './frame_processor.js';The new exports isFrameProcessor, isAudioFrameProcessor, isVideoFrameProcessor, FrameProcessorSymbol, and FrameProcessorType are missing from this block. External consumers who want to use the type guards (the primary feature of this PR) will be unable to import them from the package.
Impact: The main feature of the PR is inaccessible to external users. They would have to resort to deep imports from internal module paths, which is fragile and not part of the public API contract.
View 6 additional findings in Devin Review.
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
🐛 1 issue in files not directly in the diff
🐛 New type guards and symbols not exported from package public API (packages/livekit-rtc/src/index.ts:53-57)
The new type guard functions (isFrameProcessor, isAudioFrameProcessor, isVideoFrameProcessor), FrameProcessorSymbol, and FrameProcessorType are defined in frame_processor.ts but not re-exported from index.ts, which is the package's public entry point.
Root Cause and Impact
The entire purpose of this PR is to provide type guards to avoid the dual package hazard with instanceof checks. However, consumers of the @livekit/rtc-node package cannot access these new utilities because they are not exported from packages/livekit-rtc/src/index.ts:53-57:
export {
FrameProcessor,
type FrameProcessorStreamInfo,
type FrameProcessorCredentials,
} from './frame_processor.js';The new exports isFrameProcessor, isAudioFrameProcessor, isVideoFrameProcessor, FrameProcessorSymbol, and FrameProcessorType are missing from this block. External consumers who want to use the type guards (the primary feature of this PR) will be unable to import them from the package.
Impact: The main feature of the PR is inaccessible to external users. They would have to resort to deep imports from internal module paths, which is fragile and not part of the public API contract.
View 9 additional findings in Devin Review.
| } | ||
|
|
||
| export abstract class FrameProcessor<Frame extends VideoFrame | AudioFrame> { | ||
| readonly symbol = FrameProcessorSymbol; |
There was a problem hiding this comment.
todo: Since think this means that it won't work with frame processors defined with old @livekit/rtc-node versions, make sure you don't forget to update the peerDependency version in the aic package after you've tested all this locally and are ready to release your changes!
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
🐛 1 issue in files not directly in the diff
🐛 New type guards and symbols not exported from package public API (packages/livekit-rtc/src/index.ts:53-57)
The new type guard functions (isFrameProcessor, isAudioFrameProcessor, isVideoFrameProcessor), FrameProcessorSymbol, and FrameProcessorType are defined in frame_processor.ts but not re-exported from index.ts, which is the package's public entry point.
Root Cause and Impact
The entire purpose of this PR is to provide type guards to avoid the dual package hazard with instanceof checks. However, consumers of the @livekit/rtc-node package cannot access these new utilities because they are not exported from packages/livekit-rtc/src/index.ts:53-57:
export {
FrameProcessor,
type FrameProcessorStreamInfo,
type FrameProcessorCredentials,
} from './frame_processor.js';The new exports isFrameProcessor, isAudioFrameProcessor, isVideoFrameProcessor, FrameProcessorSymbol, and FrameProcessorType are missing from this block. External consumers who want to use the type guards (the primary feature of this PR) will be unable to import them from the package.
Impact: The main feature of the PR is inaccessible to external users. They would have to resort to deep imports from internal module paths, which is fragile and not part of the public API contract.
View 10 additional findings in Devin Review.
…sdks into lukas/processor-typeguards
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
🐛 1 issue in files not directly in the diff
🐛 New type guards and symbols not exported from package public API (packages/livekit-rtc/src/index.ts:53-57)
The new type guard functions (isFrameProcessor, isAudioFrameProcessor, isVideoFrameProcessor), FrameProcessorSymbol, and FrameProcessorType are defined in frame_processor.ts but not re-exported from index.ts, which is the package's public entry point.
Root Cause and Impact
The entire purpose of this PR is to provide type guards to avoid the dual package hazard with instanceof checks. However, consumers of the @livekit/rtc-node package cannot access these new utilities because they are not exported from packages/livekit-rtc/src/index.ts:53-57:
export {
FrameProcessor,
type FrameProcessorStreamInfo,
type FrameProcessorCredentials,
} from './frame_processor.js';The new exports isFrameProcessor, isAudioFrameProcessor, isVideoFrameProcessor, FrameProcessorSymbol, and FrameProcessorType are missing from this block. External consumers who want to use the type guards (the primary feature of this PR) will be unable to import them from the package.
Impact: The main feature of the PR is inaccessible to external users. They would have to resort to deep imports from internal module paths, which is fragile and not part of the public API contract.
View 8 additional findings in Devin Review.
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
🐛 1 issue in files not directly in the diff
🐛 New type guards and symbols not exported from package public API (packages/livekit-rtc/src/index.ts:53-57)
The new type guard functions (isFrameProcessor, isAudioFrameProcessor, isVideoFrameProcessor), FrameProcessorSymbol, and FrameProcessorType are defined in frame_processor.ts but not re-exported from index.ts, which is the package's public entry point.
Root Cause and Impact
The entire purpose of this PR is to provide type guards to avoid the dual package hazard with instanceof checks. However, consumers of the @livekit/rtc-node package cannot access these new utilities because they are not exported from packages/livekit-rtc/src/index.ts:53-57:
export {
FrameProcessor,
type FrameProcessorStreamInfo,
type FrameProcessorCredentials,
} from './frame_processor.js';The new exports isFrameProcessor, isAudioFrameProcessor, isVideoFrameProcessor, FrameProcessorSymbol, and FrameProcessorType are missing from this block. External consumers who want to use the type guards (the primary feature of this PR) will be unable to import them from the package.
Impact: The main feature of the PR is inaccessible to external users. They would have to resort to deep imports from internal module paths, which is fragile and not part of the public API contract.
View 11 additional findings in Devin Review.
2 participants
Instead of relying on unreliable
instanceofchecks we introduce a dedicated symbol and type to make it both easy to differentiate which kind of processor and also whether or not it is one in the first place.