Skip to content
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.

Sign up for GitHub

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

Jump to bottom

VIDEO-8939 - Migrating twilio/webrtc #1731

Merged
merged 15 commits into from
Mar 16, 2022
Merged

VIDEO-8939 - Migrating twilio/webrtc #1731

merged 15 commits into from
Mar 16, 2022

Conversation

PikaJoyce
Copy link
Contributor

JIRA Ticket: VIDEO-8939

This PR is for the first phase of webrtc files being migrated over within the SDK. This should subsequently fix VIDEO-8609 as well.

I have verified that webpack 5 will compile the sdk successfully after folding in twilio/webrtc. See image below
image

Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

  • I acknowledge that all my contributions will be made under the project's license.

Pull Request Details

Burndown

Before review

  • Updated CHANGELOG.md if necessary
  • Added unit tests if necessary
  • Updated affected documentation
  • Verified locally with npm test
  • Manually sanity tested running locally
  • Included screenshot as PR comment (if needed)
  • Ready for review

@PikaJoyce PikaJoyce self-assigned this Mar 14, 2022
@makarandp0
Copy link
Contributor

makarandp0 commented Mar 14, 2022 •
edited
Loading

This is great news @PikaJoyce - Did the fix require using separate modules for util and events?

Since this PR adds tons of new files, I think its hard to review as it is, we mainly want to review any edits that you made to the twilio/webrtc files, and any existing twilio-video files. Can you split this into two commits

commit 1: just copy over all webrtc files. We do not need to review this part, and Its okay if the build does not work after this commit.
commit 2: any edits on top of commit 1.

@makarandp0
Copy link
Contributor

Nice @PikaJoyce!
I think with twilio/webrtc moving over, we need to reorganize tests - not all webrtc tests need to run as part of every PR (and not all tests need to run in every combination of browser/bver) But that can be done as a separate PR - Please file a task for this.

but this PR looks good to me.

@PikaJoyce PikaJoyce merged commit 28206a9 into develop Mar 16, 2022
@PikaJoyce PikaJoyce deleted the VIDEO-8939-Migrate-TwilioWebRTC-lib branch March 16, 2022 23:52
PikaJoyce added a commit that referenced this pull request Mar 22, 2022
@PikaJoyce @charliesantos
@manjeshbhargav @twilio-ci @makarandp0
Edit changelog and readme for 2.21.1 release (#1743)
664d6e2 
* Merge Preflight and PlanB removal feature branch (#1702)

* VIDEO-7728 | Better Preflight Errors (#1689)

* Adding better errors on preflight

* lint

* Update docs

* Changelog

* Update docs

* Convert error to string

* Adding timestamp to progress events

* Properly raising signaling errors

* lint

* Update tests

* Mak's feedback

* Update test

* Feature/remove plan b (#1697)

* VIDEO-6587 | Remove Plan B in SDK (#1656)

* Initial implementation.

* Fixing integration tests

* Updating unit tests

* Adding back unit test job

* Removing unneeded case

* Fix build

* Adding rename suggestion

Co-authored-by: Manjesh Malavalli <[email protected]>

* Adding changelog

* Update CHANGELOG.md

Co-authored-by: Manjesh Malavalli <[email protected]>

* Adding ticket number

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>

* Move changelog to 2.20.0 section

* 2.20.0-rc1

* 2.20.0-dev

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>

* Fix unit test from merge

* 2.20.0-rc2

* 2.20.0-dev

* Update doc

* Update changelog

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>

* Removing framework tests (#1700)

* Convert createLocalTracks to TS (#1694)

* VIDEO-7714 - Convert createLocalTracks to TypeScript

Co-authored-by: joma <[email protected]>
Co-authored-by: charliesantos <[email protected]>

* Update @twilio/webrtc to 4.6.0-rc1 (#1705)

* Update @twilio-webrtc to RC

Co-authored-by: joma <[email protected]>

* 2.21.0-rc1

* 2.21.0-dev

* merge from master

* VIDEO-8791 - Adding extra information into CHANGELOG (#1716)

* Adding extra information into CHANGELOG

* 2.21.0-rc2

* 2.21.0-dev

* VIDEO-8647 - Fixing Chrome Docker Image (#1718)

* VIDEO-8647 Fixing docker images build for Chrome

* VIDEO-8954: Adding in extra links and details to the CHANGELOG entry (#1722)

* VIDEO-8954 - Adding in extra links and details to the CHANGELOG entry

* VIDEO-8647 - Fixing Chrome Docker Image (#1718) (#1721) (#1728)

* VIDEO-8647 Fixing docker images build for Chrome

* VIDEO-8939 - Migrating twilio/webrtc (#1731)

* VIDEO-8939 - Folding twilio/webrtc into SDK, installing util and events packages.

* Prep for 2.21.1 RC (#1739)

* Adding CHANGELOG entry for 2.21.1 RC

* 2.21.1-rc1

* 2.21.1-dev

* Edit changelog and readme for 2.21.1 release

Co-authored-by: Charlemagne Santos <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>
Co-authored-by: Makarand Patwardhan <[email protected]>
@xtianjohns
Copy link

xtianjohns commented Mar 28, 2022 •
edited
Loading

Hey @makarandp0, @PikaJoyce, @charliesantos just a note here - I don't think that the events and util polyfill problem is solved. I think for webpack this can likely work because transforms will supply the correct browser shims for them.

However, for rollup, this is still a major problem. I think this could be being tracked in VIDEO-6516 but I'm not really sure. Github issue is #1347. Until twilio-video stops depending on the polyfills or some other solution is reached, the only way to bundle the SDK into some other output format is to use webpack.

@PikaJoyce
Copy link
Contributor Author

Hi @xtianjohns!

Thanks for the comment. We realize that for snowpack/rollup that this issue still exists. We currently have a ticket to address this specific issue (VIDEO-3612). We hope to resolve this issue for all users soon.

We appreciate your patience! Thank you!

Best,
Joyce

PikaJoyce added a commit that referenced this pull request Jun 1, 2022
@PikaJoyce @charliesantos
@manjeshbhargav @twilio-ci @makarandp0 @timmydoza
Prep release 2.21.2 (#1792)
ad2cede 
* Merge Preflight and PlanB removal feature branch (#1702)

* VIDEO-7728 | Better Preflight Errors (#1689)

* Adding better errors on preflight

* lint

* Update docs

* Changelog

* Update docs

* Convert error to string

* Adding timestamp to progress events

* Properly raising signaling errors

* lint

* Update tests

* Mak's feedback

* Update test

* Feature/remove plan b (#1697)

* VIDEO-6587 | Remove Plan B in SDK (#1656)

* Initial implementation.

* Fixing integration tests

* Updating unit tests

* Adding back unit test job

* Removing unneeded case

* Fix build

* Adding rename suggestion

Co-authored-by: Manjesh Malavalli <[email protected]>

* Adding changelog

* Update CHANGELOG.md

Co-authored-by: Manjesh Malavalli <[email protected]>

* Adding ticket number

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>

* Move changelog to 2.20.0 section

* 2.20.0-rc1

* 2.20.0-dev

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>

* Fix unit test from merge

* 2.20.0-rc2

* 2.20.0-dev

* Update doc

* Update changelog

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>

* Removing framework tests (#1700)

* Convert createLocalTracks to TS (#1694)

* VIDEO-7714 - Convert createLocalTracks to TypeScript

Co-authored-by: joma <[email protected]>
Co-authored-by: charliesantos <[email protected]>

* Update @twilio/webrtc to 4.6.0-rc1 (#1705)

* Update @twilio-webrtc to RC

Co-authored-by: joma <[email protected]>

* 2.21.0-rc1

* 2.21.0-dev

* merge from master

* VIDEO-8791 - Adding extra information into CHANGELOG (#1716)

* Adding extra information into CHANGELOG

* 2.21.0-rc2

* 2.21.0-dev

* VIDEO-8647 - Fixing Chrome Docker Image (#1718)

* VIDEO-8647 Fixing docker images build for Chrome

* VIDEO-8954: Adding in extra links and details to the CHANGELOG entry (#1722)

* VIDEO-8954 - Adding in extra links and details to the CHANGELOG entry

* VIDEO-8647 - Fixing Chrome Docker Image (#1718) (#1721) (#1728)

* VIDEO-8647 Fixing docker images build for Chrome

* VIDEO-8939 - Migrating twilio/webrtc (#1731)

* VIDEO-8939 - Folding twilio/webrtc into SDK, installing util and events packages.

* Prep for 2.21.1 RC (#1739)

* Adding CHANGELOG entry for 2.21.1 RC

* 2.21.1-rc1

* 2.21.1-dev

* Marking unstable integration tests (#1740)

* Marking unstable integration tests

* merge from master (#1745)

* VIDEO-9511 - do not configure encodings for stopped tracks (#1768)

* do not update encodings for stopped tracks

* Emit dimensionsChanged event on videoTrack start

* Add test for videoTrack _start method

* Add changelog entry for VIDEO-3576

* VIDEO-9282 - Migrate SDK from using node.js util. (#1752)

* VIDEO-9282 - Removing all references to node dependencies.

Co-authored-by: Makarand Patwardhan <[email protected]>

* 2.21.2-rc1

* 2.21.2-dev

* VIDEO-8282 - Adding iPad Detection (#1779)

* VIDEO-8282 - Adding iPad and iPhone detection logic

* 2.21.2-rc2

* 2.21.2-dev

* Prep for 2.21.2 release, updating changelog

Co-authored-by: Charlemagne Santos <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>
Co-authored-by: Makarand Patwardhan <[email protected]>
Co-authored-by: Tim Mendoza <[email protected]>
Co-authored-by: timmydoza <[email protected]>
charliesantos added a commit that referenced this pull request Jun 7, 2022
@charliesantos @manjeshbhargav
@twilio-ci @PikaJoyce @makarandp0 @timmydoza
2.21.3 prep (#1804)
ccd9b55 
* Merge Preflight and PlanB removal feature branch (#1702)

* VIDEO-7728 | Better Preflight Errors (#1689)

* Adding better errors on preflight

* lint

* Update docs

* Changelog

* Update docs

* Convert error to string

* Adding timestamp to progress events

* Properly raising signaling errors

* lint

* Update tests

* Mak's feedback

* Update test

* Feature/remove plan b (#1697)

* VIDEO-6587 | Remove Plan B in SDK (#1656)

* Initial implementation.

* Fixing integration tests

* Updating unit tests

* Adding back unit test job

* Removing unneeded case

* Fix build

* Adding rename suggestion

Co-authored-by: Manjesh Malavalli <[email protected]>

* Adding changelog

* Update CHANGELOG.md

Co-authored-by: Manjesh Malavalli <[email protected]>

* Adding ticket number

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>

* Move changelog to 2.20.0 section

* 2.20.0-rc1

* 2.20.0-dev

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>

* Fix unit test from merge

* 2.20.0-rc2

* 2.20.0-dev

* Update doc

* Update changelog

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>

* Removing framework tests (#1700)

* Convert createLocalTracks to TS (#1694)

* VIDEO-7714 - Convert createLocalTracks to TypeScript

Co-authored-by: joma <[email protected]>
Co-authored-by: charliesantos <[email protected]>

* Update @twilio/webrtc to 4.6.0-rc1 (#1705)

* Update @twilio-webrtc to RC

Co-authored-by: joma <[email protected]>

* 2.21.0-rc1

* 2.21.0-dev

* merge from master

* VIDEO-8791 - Adding extra information into CHANGELOG (#1716)

* Adding extra information into CHANGELOG

* 2.21.0-rc2

* 2.21.0-dev

* VIDEO-8647 - Fixing Chrome Docker Image (#1718)

* VIDEO-8647 Fixing docker images build for Chrome

* VIDEO-8954: Adding in extra links and details to the CHANGELOG entry (#1722)

* VIDEO-8954 - Adding in extra links and details to the CHANGELOG entry

* VIDEO-8647 - Fixing Chrome Docker Image (#1718) (#1721) (#1728)

* VIDEO-8647 Fixing docker images build for Chrome

* VIDEO-8939 - Migrating twilio/webrtc (#1731)

* VIDEO-8939 - Folding twilio/webrtc into SDK, installing util and events packages.

* Prep for 2.21.1 RC (#1739)

* Adding CHANGELOG entry for 2.21.1 RC

* 2.21.1-rc1

* 2.21.1-dev

* Marking unstable integration tests (#1740)

* Marking unstable integration tests

* merge from master (#1745)

* VIDEO-9511 - do not configure encodings for stopped tracks (#1768)

* do not update encodings for stopped tracks

* Emit dimensionsChanged event on videoTrack start

* Add test for videoTrack _start method

* Add changelog entry for VIDEO-3576

* VIDEO-9282 - Migrate SDK from using node.js util. (#1752)

* VIDEO-9282 - Removing all references to node dependencies.

Co-authored-by: Makarand Patwardhan <[email protected]>

* 2.21.2-rc1

* 2.21.2-dev

* VIDEO-8282 - Adding iPad Detection (#1779)

* VIDEO-8282 - Adding iPad and iPhone detection logic

* 2.21.2-rc2

* 2.21.2-dev

* Added details in changelog entry and updated insights payload constants (#1789)

* 2.21.2-rc3

* 2.21.2-dev

* Merge from master, resolving conflicts, adding new changelog entry (#1796)

* VIDEO-10199 Update tooling back to working versions (#1798)

* Adding platformName into insights payload (#1799)

* Changelog for RC

* 2.21.3-rc1

* 2.21.3-dev

* Update changelog and readme

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>
Co-authored-by: Joyce Ma <[email protected]>
Co-authored-by: joma <[email protected]>
Co-authored-by: Makarand Patwardhan <[email protected]>
Co-authored-by: Tim Mendoza <[email protected]>
Co-authored-by: timmydoza <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@makarandp0 makarandp0 makarandp0 approved these changes

@manjeshbhargav manjeshbhargav Awaiting requested review from manjeshbhargav

@charliesantos charliesantos Awaiting requested review from charliesantos

Assignees

@PikaJoyce PikaJoyce

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@PikaJoyce @makarandp0 @xtianjohns