daily-co / daily-js

https://docs.daily.co/reference/daily-js
BSD 2-Clause "Simplified" License
103 stars 33 forks source link

.destroy() never resolves if called inside left-meeting event handler #185

Closed frankie567 closed 2 years ago

frankie567 commented 2 years ago

Expected behavior

Inside a left-meeting event handler, I want to be able to call .destroy() so that the resources and the iframe are destroyed after the participant has left.

Describe the bug (unexpected behavior)

When calling .destroy(), the promise is never resolved and the resources and iframe are not destroyed.

Steps to reproduce

CodeSandbox reproducing the issue

const callIFrame = DailyIframe.createFrame({
    iframeStyle: {
        position: "fixed",
        top: "0px",
        left: "0px",
        width: "100%",
        height: "100%"
    },
    showLeaveButton: true,
    activeSpeakerMode: false
});
callIFrame.join({
    url: "XXX"
});

callIFrame.on("left-meeting", () => {
    console.log("Call destroy...");
    callIFrame.destroy().then(() => {
        console.log("Destroyed!"); // Never reached
    });
});

System information

* Device: MacBook Pro M1 * OS, version: macOS 12.4 * Browser, version: Google Chrome 101 * @daily-co/daily-js: 0.26.0 # Additional context * It was working well in version `@daily-co/daily-js 0.22.0`. * I suspect it was introduced by this change: dff9d8259d819e90c49be398b6edd02f2fa0ddef
mklepaczewski commented 2 years ago

We're also affected by the issue. Daily team - any chance it's going to be fixed soon?

frankie567 commented 2 years ago

This is still happening with @daily-co/daily-js 0.31.0 (I updated the CodeSandbox).

This issue is a bit annoying when you want to show another page in your SPA after the participant has left the meeting.

For the record, here is the temporary fix I implemented. Basically, I'm manually removing the iframe from the DOM (Daily will complain in the console though because it can't find it anymore):

const onLeftMeeting = useCallback(
  async () => {
    if (callObject) {
      await callObject.stopRecording();
      // FIXME: Temporary workaround. See https://github.com/daily-co/daily-js/issues/185
      const iframe = callObject.iframe();
      if (iframe) {
        iframe.parentElement?.removeChild(iframe);
      }
    }
  },
  [callObject],
);
mattieruth commented 2 years ago

Thank you for the report and pings on this! I have added this to my list of things to look into this week and will get back to y'all.

mattieruth commented 2 years ago

Update: I have merged a fix for this issue. Acquiring the fix will require a version upgrade to 0.32.0 once it is released which should be some time this week. I will close this issue when that happens. Thanks again!

mattieruth commented 2 years ago

This has now been released. Closing.