Skip to content

Conversation

@robintown
Copy link
Member

@robintown robintown commented Apr 8, 2025

This makes the mechanism by which an Element Call widget can receive changes to room state more reliable by updating matrix-js-sdk and matrix-widget-api, which now support the update_state action from the latest version of MSC2762.

Depends on matrix-org/matrix-js-sdk#4790
For https://github.com/element-hq/voip-internal/issues/289

@robintown robintown force-pushed the robin/reintroduce-update-state branch from 7924b3a to e81dd84 Compare May 5, 2025 15:29
@robintown robintown changed the title Update matrix-js-sdk to support update_state action Update matrix-js-sdk, matrix-widget-api to support update_state action May 5, 2025
@robintown robintown added the PR-Improvement Release note category. A PR that improves EC's performance or stability. label May 5, 2025
@robintown robintown changed the title Update matrix-js-sdk, matrix-widget-api to support update_state action Improve the reliability of state changes in widget mode May 5, 2025
@robintown robintown force-pushed the robin/reintroduce-update-state branch from e81dd84 to c473d1f Compare May 20, 2025 21:55
@robintown robintown marked this pull request as ready for review May 20, 2025 21:55
@robintown robintown requested a review from a team as a code owner May 20, 2025 21:55
@robintown robintown requested a review from BillCarsonFr May 20, 2025 21:55
Copy link
Contributor

@toger5 toger5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a simple patch PR should look like.
I am wondering if we can improve on the "custom js-sdk hash" situation?

package.json Outdated
"loglevel": "^1.9.1",
"matrix-js-sdk": "github:matrix-org/matrix-js-sdk#f3f4a1f7e2f5f2c26cf4fce89a729a9197f619b5",
"matrix-widget-api": "1.11.0",
"matrix-js-sdk": "github:matrix-org/matrix-js-sdk#7fa9195e398764749b0195f14d3cf0190ad10253",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this particular instance it might make sense to mention where exactly the js-sdk commit comes from.

I would almost argue we should make this a procedure rule:

  • If one uses a commit NOT from the develop (main) branch, a link to the related branch with a short summary has to be added as a comment.
Suggested change
"matrix-js-sdk": "github:matrix-org/matrix-js-sdk#7fa9195e398764749b0195f14d3cf0190ad10253",
"NONMAIN-matrix-js-sdk":"This commit is from: https://github.com/matrix-org/matrix-js-sdk/pull/4847, the PR combines state-after and widget driver to-device fallback featurres.",
"matrix-js-sdk": "github:matrix-org/matrix-js-sdk#7fa9195e398764749b0195f14d3cf0190ad10253",

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot its not that simple. NONMAIN-... will raise an error as the npm package cannot be found.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we might need to find a good place somewhere else. package.json.md?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Different, possibly simpler idea:

Yarn has the ability to reference a Git dependency by branch name rather than commit. We can just use that! Then it's not too difficult to observe that we're using a non-develop branch and go over to https://github.com/matrix-org/matrix-js-sdk to dig into the details of the branch if you really need them.

e4924df

I think Yarn has always had this feature, but in Yarn Classic it caused the package cache to be a bit broken and that's why we always avoided it. But now we're of course on Yarn Berry.

@toger5 toger5 added the backport-candidate Something that is a candidate for backport to a particular release branch label May 21, 2025
Rather than by commit, which makes it hard to tell whether we're using mainline matrix-js-sdk or not.
@robintown robintown added X-Blocked Cannot be merged due to external dependencies and removed X-Blocked Cannot be merged due to external dependencies labels May 21, 2025
@robintown robintown enabled auto-merge May 21, 2025 17:08
@robintown robintown merged commit aeae964 into livekit May 21, 2025
27 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-candidate Something that is a candidate for backport to a particular release branch PR-Improvement Release note category. A PR that improves EC's performance or stability.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants