refactor(useMediaControls): Deprecate options that can simply be set as attributes

[cawa-93]

If you change the volume or muted values by native controls (or in devtools) it does not effect reactive variables. Here is a demonstration.

[wheatjs]

Good catch, I actually noticed these issues recently but hadn't had the chance to go back and fix them. Thanks for picking up my slack on these 😄

[wheatjs]

Maybe consider adding a check if el exists to get rid of the error

[cawa-93]

I hastened with it. How could we update the mute that comes from options? My first think was:

    if (options.muted !== undefined && isRef(options.muted))
      options.muted.value = (unref(target))!.muted

But TS prompts me that options.muted.value is readonly.

UseMediaControlsOptions.muted: Ref<boolean> | ComputedRef<boolean>

[wheatjs]

Hmm, I'll take a look tomorrow if you don't figure it out before then.

[cawa-93]

What if return new muted value? In the same way as all other parameters return.

{
  muted // <-- Reactive value
} = useMediaControls(video, {
  muted: MaybeRef<boolean>, // <-- Initial value
}

[wheatjs]

Hmm, maybe we should just remove the volume and muted objects from the initial parameters all together? Maybe things like muted and volume should just be returned from the object like you suggested. I feel like having the initial parameters and also returned might be over complicating it. I think what would be best is things that have a 2 way data flow, like volume, currentTime, and muted are returned from the function while things that have a one-way data flow like poster should be treated as configuration options.

[cawa-93]

Sounds good.

How about to deprecate one-way data that can be set by <video> attibutes and does't have any "reactive logic": loop, controls, preload, autoplay, poster, playsInline and autoPictureInPicture.

Don't

const poster = ref('[link to poster image]')
const {} = useMediaControls(target, {
  poster,
})

Do

<template>
  <video :poster="poster">
</template>

<script setup>
const poster = ref('[link to poster image]')

const {} = useMediaControls(target, {})
</script>

What do you thinks?

UPD.
At first glance, this looks like a compatible change, because after v4.11 we can already use the attributes to configure <video>. So all we have to do is add an deprecation message and remove that options in v5.0

[wheatjs]

Hmm yeah that seems like a decent idea, doing less might be a better idea in this case. We mostly want to simplify that more difficult to control portions of the video element which I think we already do pretty well. So some of the simpler options like poster can be removed from the function in favor of just using the element attributes.

[cawa-93]

@wheatjs Please review how I create deprecation message. If all goes well, I'll do the same for the rest of the options I talked about earlier.

[wheatjs]

@wheatjs Please review how I create deprecation message. If all goes well, I'll do the same for the rest of the options I talked about earlier.

Sure thing, sorry for the delay, I'll review this sometime today 😄

[antfu]

How about to deprecate one-way data that can be set by <video> attibutes and does't have any "reactive logic": loop, controls, preload, autoplay, poster, playsInline and autoPictureInPicture.

Yeah, that sounds better to me to be as well. User could also use v-bind when they want to provide them altogether as an object.

[wheatjs]

@wheatjs Please review how I create deprecation message. If all goes well, I'll do the same for the rest of the options I talked about earlier.

The deprecation messages look good to me. Once you add them for the rest of the functions we can do another review and make sure everything works like expected

[cawa-93]

I don't changed demo in last commit for checking backward compatibility

[wheatjs]

Nice job with the console warns as well, it looks good too me

[antfu]

I was trying to avoid console logs in VueUse whenever possible. Since we don't have different builds for dev and prod, using consoles would also ships extra bits and hard-coded strings to production. Which are also out of users' control

I would suggest to mark the API depredated on type level but keep the behavior the same, and then we can remove the code in major versions later

[wheatjs]

Nice job with the console warns as well, it looks good too me

Sorry I forgot about us trying to avoid console logs 😅

[wheatjs]

This is only slightly relevant to this PR, but it seems like my links for the subtitles expired for the demo so if you want you can add them locally maybe? Otherwise we can do it in another PR

Here are the gists for the subtitles:

English https://gist.github.com/wheatjs/a85a65a82d87d7c098e1a0972ef1f726
French https://gist.github.com/wheatjs/38f32925d20c683bf77ba33ff737891b