fix(recent-list): prefer conference URL on CONFERENCE_WILL_LEAVE#17283
fix(recent-list): prefer conference URL on CONFERENCE_WILL_LEAVE#17283Subhajit578 wants to merge 2 commits intojitsi:masterfrom
Conversation
…types - Use JITSI_CONFERENCE_URL_KEY from leaving conference, fallback to connection - Update forEachConference comment; add ExcalidrawAppProps augmentation for tsc Made-with: Cursor
|
While validating the change, I also hit a separate TypeScript issue when running: |
|
Hi, thanks for your contribution! |
| * Ensures augmentation merges with the real `@jitsi/excalidraw` typings (see | ||
| * TypeScript handbook: module augmentation). | ||
| */ | ||
| import type {} from '@jitsi/excalidraw'; |
There was a problem hiding this comment.
What is this and how is it related to the PR?
There was a problem hiding this comment.
npm run tsc:web was failing after I pulled the recent changes. I looked into it and it was a typing mismatch.
Deatiled Problem - Whiteboard.tsx and WhiteboardWrapper.tx pass meetingDetails, jwt and strogareBackendUrl to Excalidraw Prop but those props are not declared in ExcalidrawAppProps which causes this. This is just a local typescript module augmentation to make the existing runtime usage match the package typings. So it is not related to the fix - recent-list behaviour fix so I can remove it if needed and work on that separately and raise a pr with the long term fix as I said in the earlier comment.
There was a problem hiding this comment.
Hi @saghul, could you take a look when you get a chance? Happy to drop the whiteboard typing change from this PR and open a separate one PR for it if that's preferred.
There was a problem hiding this comment.
Looks like I was getting the issue because I was using a older node version when I updated it it ran perfectly fine. So I have removed [jitsi-excalidraw-augment.d.ts]which was introduced by me to fix the type mismatch error. Could you please review the changes when you get the time. Thank You!
|
jenkins please test this please. |
|
I looked through the Allure report, it failed because of media-setup timeouts (waitForReceiveMedia / p1) and I have not touched that part. So I think This can be a flaky test. Can re-run if needed though. |
When a meeting ends, recent-list updates the stored session duration by matching on a URL. The problem is that web was using features/base/connection.locationURL for this — but that's global connection state. If you leave a meeting and navigate somewhere else quickly, locationURL can already point to the new room before the leave event fires, so the duration ends up stamped on the wrong entry.
Native was already doing this correctly using conference[JITSI_CONFERENCE_URL_KEY], which is scoped to the specific conference being torn down.
Fix -
Align web with native by resolving the URL in this order:
conference?.[JITSI_CONFERENCE_URL_KEY] — tied to the conference actually being left
Fall back to features/base/connection.locationURL if that's not available
Skip the dispatch entirely if neither resolves to a valid href
This closes the race condition and makes the behavior consistent across platforms. Also clears the FIXME in react/features/recent-list/middleware.ts.
Also changed
Cleaned up the forEachConference JSDoc in react/features/base/conference/functions.ts to match what it actually does now.
Testing
npm run tsc:web ran without error
npm run tsc:ci ran without error
npm run lint:ci ran without error
Left a meeting and immediately jumped to a new room — the duration landed on the correct recent entry each time.