`useCallFrame` creates an iframe directly inside `<body>` ignoring the component structure

ahkhanjani commented 9 months ago

Please tell me if I'm doing something wrong. Here's my simplified code:

function Call({ roomUrl, meetingToken }: { roomUrl: string; meetingToken: string | undefined }) {
  const callRef = useRef<HTMLDivElement>(null);
  const callFrame = useCallFrame({
    parentEl: callRef.current,
    options: CALL_OPTIONS,
    shouldCreateInstance: useCallback(() => Boolean(callRef.current), []),
  });

  useEffect(() => {
    if (!callFrame) {
      return;
    }

    void callFrame.join({ url: roomUrl, token: meetingToken });

    callFrame.on("left-meeting", () => {
      void callFrame.destroy();
    });
  }, [callFrame, meetingToken, roomUrl]);

  return (
    <DailyProvider callObject={callFrame}>
      <div ref={callRef} className="h-full w-full" />
    </DailyProvider>
  );
}

Deleting shouldCreateInstance or the <div> which callRef is passed into doesn't make a difference; they're basically ignored.

Package versions:

"@daily-co/daily-js": "0.57.4",
"@daily-co/daily-react": "^0.17.1"

Regaddi commented 9 months ago

Hey @ahkhanjani,

thanks a lot for the report!

I think the issue is that callRef.current and the useCallback passed to shouldCreateInstance don't differ between render cycles from React's "what changed" perspective. Testing your code I'm noticing that callRef.current initially is null (correct), but as soon as the first batch of side-effects runs (essentially the useEffect() checking for shouldCreateInstance()) the ref is already available, but not added to the DOM, yet (sorry, this is a bit of a hand-wavy explanation).

I can definitely see that you just followed the example from our docs, so those definitely need to be updated.

The iframe is correctly mounted when the code is dependent on a React state, rather than just a ref.

Here's your adjusted code example that at least works for me in React 18:

function Call({ roomUrl, meetingToken }: { roomUrl: string; meetingToken: string | undefined }) {
  const callRef = useRef<HTMLDivElement>(null);
  const [ready, setReady] = useState(false);
  const callFrame = useCallFrame({
    parentEl: callRef.current,
    options: CALL_OPTIONS,
    shouldCreateInstance: useCallback(() => ready, [ready]),
  });

  useEffect(() => {
    setReady(true);
  }, []);

  useEffect(() => {
    if (!callFrame) {
      return;
    }

    void callFrame.join({ url: roomUrl, token: meetingToken });

    callFrame.on("left-meeting", () => {
      void callFrame.destroy();
    });
  }, [callFrame, meetingToken, roomUrl]);

  return (
    <DailyProvider callObject={callFrame}>
      <div ref={callRef} className="h-full w-full" />
    </DailyProvider>
  );
}

I'm pretty sure there's a better alternative to the ready state.

ahkhanjani commented 9 months ago

Thank you @Regaddi. This indeed fixed the issue.

I'm pretty sure there's a better alternative to the ready state.

Any chances this can be handled internally?

Regaddi commented 9 months ago

I have a few ideas that I want to try out! I'll let you know about any progress 👍

Regaddi commented 9 months ago

@ahkhanjani This should be improved in 0.17.2.

daily-co / daily-react

`useCallFrame` creates an iframe directly inside `<body>` ignoring the component structure #30