Bug: React warning: `maxResolution` being passed to DOM Element

sabrichu commented 9 months ago

Is there an existing issue for this?

[X] I have searched the existing issues

Which Mux Elements/Packages does this apply to? Select all that apply

mux-video-react

Which browsers are you using?

Chrome

Which operating systems are you using?

macOS

Description

This is a similar bug to https://github.com/muxinc/elements/issues/602 in that Mux video props like maxResolution etc are being spread to the underlying <video> element, causing React warnings. Some props like streamType are being plucked out now before ...rest gets spread, but it's an incomplete list.

https://github.com/muxinc/elements/blob/main/packages/mux-video-react/src/index.tsx#L27

An ideal fix would stop all props meant to only be passed as Mux media props from being spread to <video> so that there aren't similar future issues filed.

Reduced test case

No response

Steps to reproduce

Create a mux-video react component with maxResolution="1080p", e.g:

import MuxVideo from '@mux/mux-video-react';

const VideoPlayer = () => 
   <MuxVideo
        maxResolution="1080p"
        {...otherProps}
    />

Current Behavior

Warning logged in console

Expected Behavior

No warning logged

Errors

What version of the package are you using?

0.8.3

cjpillsbury commented 8 months ago

Thanks for filing the bug. We've added this feature to <mux-video> (web component), <mux-player> (web component), and <MuxPlayer> (react), but haaven't yet added it to <MuxVideo> (react). While these will be passed down to our "playback-core" and thus get applied "automagically", you're encountering some of the rough edges that result from not having official support just yet. As a stopgap, could you instead directly construct the src url, e.g.

src={`https:stream.mux.com/${playbackId}.m3u8?max_resolution=1080p`}

(See: https://docs.mux.com/guides/control-playback-resolution#specify-maximum-resolution for examples from the docs)

I'll also be sure to keep you posted/update this issue if/when we get a chance to tackle this in our priority queue.

sabrichu commented 8 months ago

Thanks for filing the bug. We've added this feature to <mux-video> (web component), <mux-player> (web component), and <MuxPlayer> (react), but haaven't yet added it to <MuxVideo> (react). While these will be passed down to our "playback-core" and thus get applied "automagically", you're encountering some of the rough edges that result from not having official support just yet. As a stopgap, could you instead directly construct the src url, e.g.
src={`https:stream.mux.com/${playbackId}.m3u8?max_resolution=1080p`}
(See: https://docs.mux.com/guides/control-playback-resolution#specify-maximum-resolution for examples from the docs)

I'll also be sure to keep you posted/update this issue if/when we get a chance to tackle this in our priority queue.

Sounds good, thank you for the workaround!

muxinc / elements