-
Notifications
You must be signed in to change notification settings - Fork 354
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: update internal-media-core version #4010
fix: update internal-media-core version #4010
Conversation
WalkthroughThe pull request includes updates to the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/@webex/plugin-meetings/package.json (1)
65-65
: Consider updating documentation for version requirements.Since this update fixes a critical WebRTC functionality (ICE candidates), consider documenting the minimum required version in the package's README or documentation.
Would you like me to help draft the documentation update for version compatibility requirements?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (3)
packages/@webex/media-helpers/package.json
(1 hunks)packages/@webex/plugin-meetings/package.json
(1 hunks)packages/calling/package.json
(1 hunks)
🔇 Additional comments (7)
packages/@webex/media-helpers/package.json (3)
25-25
: LGTM: Version update aligns with PR objectives
The update to internal-media-core 2.12.2 aligns with the PR's goal of fixing missing ICE candidates in the SDP offer. The coordinated updates across multiple packages (@webex/plugin-meetings, @webex/calling) suggest a well-planned change.
25-25
: Verify the stability of internal-media-core v2.12.2
The version update from 2.11.3 to 2.12.2 appears to address the ICE candidates issue. Let's verify the version's stability and changes.
✅ Verification successful
Version 2.12.2 of internal-media-core is consistently applied and stable
The verification shows:
- The version 2.12.2 is consistently used across all relevant packages (calling, plugin-meetings, and media-helpers)
- The package is not deprecated on npm registry
- No breaking changes were found in the changelog related to this version
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the stability and changes of internal-media-core v2.12.2
# Check if this version is consistently used across the workspace
echo "Checking version consistency across workspace..."
rg '"@webex/internal-media-core":\s*"2\.12\.2"' -g 'package.json'
# Look for any breaking changes or migration guides
echo "Checking for breaking changes..."
fd -g CHANGELOG.md -x grep -A 10 "2.12.2"
# Verify if this version is published and stable
echo "Verifying npm registry..."
curl -s "https://registry.npmjs.org/@webex/internal-media-core" | jq '.versions."2.12.2".deprecated'
Length of output: 1977
25-27
: Verify dependency compatibility
The update also involves other media-related dependencies. Let's ensure version compatibility between internal-media-core 2.12.2 and web-media-effects 2.19.0.
✅ Verification successful
Let me gather more information about the dependency relationships between these packages.
Let me try one final verification to check the changelog or release notes for these specific versions.
Based on the gathered information, I can now provide the final response:
Dependencies appear to be safely integrated
The @webex/media-helpers
package is using both @webex/internal-media-core@2.12.2
and @webex/web-media-effects@2.19.0
. Our analysis shows:
- No explicit peer dependency requirements between these packages
- Both packages are also used together in
@webex/plugin-meetings
without version conflicts - No reported compatibility issues or breaking changes found in the changelogs
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check compatibility between media-related dependencies
# Look for any peer dependency requirements
echo "Checking peer dependencies..."
rg -A 5 "peerDependencies" -g 'package.json' --glob '**/@webex/{internal-media-core,web-media-effects}/**'
# Check for any reported issues between these versions
echo "Checking for known issues..."
gh issue list --repo webex/webex-js-sdk --label "dependencies" --search "internal-media-core web-media-effects"
Length of output: 1862
Script:
#!/bin/bash
# Check for imports and usage patterns between these packages
echo "Checking for imports and dependencies..."
rg -l "@webex/internal-media-core" packages/@webex/web-media-effects/
rg -l "@webex/web-media-effects" packages/@webex/internal-media-core/
# Check package.json files for both packages
echo "Package versions and dependencies..."
find packages/@webex/ -name "package.json" -exec grep -l "internal-media-core\|web-media-effects" {} \; -exec cat {} \;
# Look for any version constraints or compatibility notes
echo "Checking for version constraints..."
find packages/@webex/ -name "README.md" -exec grep -l "internal-media-core\|web-media-effects" {} \; -exec cat {} \;
Length of output: 5948
Script:
#!/bin/bash
# Check for any changelog entries or version compatibility notes
echo "Checking changelog entries..."
rg -i -A 5 "2.12.2|2.19.0" -g "CHANGELOG.md" .
rg -i -A 5 "compatibility" -g "CHANGELOG.md" .
# Look for any breaking changes between these versions
echo "Checking for breaking changes..."
rg -i -B 5 "BREAKING" -g "CHANGELOG.md" .
Length of output: 2601
packages/@webex/plugin-meetings/package.json (1)
65-65
: LGTM! Verify version compatibility and changelog.
The version update to address ICE candidate issues looks appropriate.
Let's verify the version compatibility and changelog:
✅ Verification successful
Version consistency verified across dependent packages
The version update to 2.12.2 is consistently applied across all relevant packages:
- packages/calling/package.json
- packages/@webex/plugin-meetings/package.json
- packages/@webex/media-helpers/package.json
No peer dependency requirements are specified for @webex/internal-media-core, indicating no version constraints from peer dependencies.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check version compatibility and changelog for @webex/internal-media-core
# Check if this version is used consistently across the workspace
echo "Checking version consistency across workspace..."
rg '"@webex/internal-media-core":\s*"2.12.2"' -g 'package.json'
# Check for any peer dependency requirements
echo "Checking package.json files for peer dependencies..."
rg '"peerDependencies"' -A 5 -g 'package.json'
Length of output: 2092
packages/calling/package.json (3)
40-40
: LGTM: Version bump looks appropriate
The minor version bump (2.11.3 → 2.12.2) is appropriate for fixing the ICE candidates issue without introducing breaking changes. The change has been tested with the JS-SDK sample application.
40-42
: Verify dependency chain compatibility
Since @webex/calling depends on both internal-media-core and media-helpers (workspace:*), we should ensure version compatibility across the dependency chain.
✅ Verification successful
Dependencies are compatible across the chain
The verification shows that both packages (@webex/calling and @webex/media-helpers) use the exact same version (2.12.2) of @webex/internal-media-core, ensuring version compatibility. No peer dependency requirements were found for these packages that could affect their compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify dependency chain compatibility
# Check media-helpers package.json for its internal-media-core version
echo "Checking media-helpers internal-media-core version:"
fd -t f package.json -x grep -l "@webex/media-helpers" {} \; | xargs cat | grep "internal-media-core"
# Check for any peer dependency requirements
echo "Checking for peer dependencies:"
fd -t f package.json -x grep -l "peerDependencies" {} \; | xargs cat
Length of output: 4797
40-40
: Verify internal-media-core version compatibility
The version bump from 2.11.3 to 2.12.2 appears to address the ICE candidates issue (SPARK-584958). Let's verify the changes and compatibility.
✅ Verification successful
The previous results show that all packages are using the same version (2.12.2), but let's verify the package-lock.json and workspace dependencies to ensure there are no version conflicts or dependency chain issues.
Version update appears consistent and safe
The version bump of @webex/internal-media-core to 2.12.2 is consistent across all dependent packages (calling, plugin-meetings, and media-helpers). All workspace dependencies are properly aligned using workspace:* references, ensuring version compatibility throughout the monorepo. The minor version bump (2.11.3 to 2.12.2) follows semver conventions and is unlikely to introduce breaking changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check version compatibility and changelog for internal-media-core
# Check if other packages in the workspace use internal-media-core
echo "Checking internal-media-core usage across workspace:"
rg '"@webex/internal-media-core":\s*"[^"]*"' --type json
# Look for potential breaking changes in commit history
echo "Checking commits between versions:"
git log --oneline --grep="internal-media-core" --since="3 months ago"
Length of output: 502
Script:
#!/bin/bash
# Let's verify the package-lock.json to ensure consistent versions
echo "Checking package-lock.json for internal-media-core versions:"
rg '"@webex/internal-media-core"' package-lock.json
# Check for any potential version conflicts in workspace dependencies
echo "Checking workspace dependencies:"
fd package.json --exec grep -l "@webex/media-helpers\|@webex/plugin-meetings" {}
Length of output: 625
Script:
#!/bin/bash
# Let's check the actual dependencies in the found package.json files
echo "Checking dependencies in webex package:"
cat ./packages/webex/package.json | grep -A5 -B5 "@webex/media-helpers\|@webex/plugin-meetings\|@webex/internal-media-core"
echo "Checking dependencies in calling package:"
cat ./packages/calling/package.json | grep -A5 -B5 "@webex/media-helpers\|@webex/plugin-meetings\|@webex/internal-media-core"
echo "Checking dependencies in plugin-meetings package:"
cat ./packages/@webex/plugin-meetings/package.json | grep -A5 -B5 "@webex/media-helpers\|@webex/internal-media-core"
echo "Checking dependencies in internal-plugin-voicea package:"
cat ./packages/@webex/internal-plugin-voicea/package.json | grep -A5 -B5 "@webex/media-helpers\|@webex/plugin-meetings"
Length of output: 2909
This pull request is automatically being deployed by Amplify Hosting (learn more). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to get a small gif or vidcast to show everything working fine?
Approving, but it would be useful to see the changes.
It will be hard to see the changes, as update of this package is meant to address the issue which is not reproducible by us. |
COMPLETES #SPARK-584958
This pull request addresses
Resolve issue with missing ice candidates in sdp offer
by making the following changes
Updates internal-media-core
Change Type
The following scenarios were tested
Tested with JS-SDK sample app if establishing media connection works
I certified that
I have read and followed contributing guidelines
I discussed changes with code owners prior to submitting this pull request
I have not skipped any automated checks
All existing and new tests passed
I have updated the documentation accordingly
Make sure to have followed the contributing guidelines before submitting.
Summary by CodeRabbit
New Features
Bug Fixes