joshwcomeau / use-sound

A React Hook for playing sound effects
MIT License
2.76k stars 98 forks source link

Allow hook options to be extended with HowlOptions #51

Closed TamoshaytisV closed 3 years ago

TamoshaytisV commented 3 years ago

Issue: https://github.com/joshwcomeau/use-sound/issues/18

Versions: Tested in my project on the following lib versions

Description: This fix does not change the default hook behavior, however, it allows to specify set of options from HowlOptions and enables IDE support for them

Usage:

import useSound from 'use-sound';
import {HowlOptions} from "howler";

import soundfile from './assets/sounds/test.ogg';

const Player = ({url, volume}: PlayerProps) => {
  const [play, {isPlaying, pause}] = useSound<HowlOptions>(url, {
    volume: volume,
    loop: true,
    onend: () => {console.log('===== ENDED');}
  });

  return <div className='player'>
    {isPlaying ?
        <PauseImg onClick={() => pause()}/> :
        <PlayImg onClick={() => play()} />
    }
  </div>
};

...

<Player url={soundfile} volume={0.2}/>

It's still usable w/o any extensions

const [play] = useSound(soundfile);

NOTE: exporting hook not only as module default will allow to augment and add overrides to the hook if needed

0xdevalias commented 3 years ago

I needed this sort of functionality in one of my projects recently (https://github.com/sparkletown/sparkle/pull/1269, ref), and ended up using the following:

export type UseCustomSoundOptions = HookOptions & HowlOptions;

Since the intended behaviour of useSound is to pass through any extra options to Howler, I wonder if it would be better if it included the TypeScript types (from @types/howler) in a similar way, within the library itself.

TimAstier commented 3 years ago

Would love to see this merged. I'll be using @0xdevalias 0xdevalias,' type for now.

TamoshaytisV commented 3 years ago

Hi guys! Thanks for your feedback! In my project, I used the latest version of @types/howler which is now exporting HowlOptions https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/howler/index.d.ts#L85 Version 2.1.2, which is used in this repo, has different typings and provides the interface HowlStatic instead So this PR is the way to combine both worlds for me, still allowing users to explicitly say what version of typings to use. I think explicit is better than implicit :) What do you think?

joshwcomeau commented 3 years ago

Sorry for the lack of response on this one — Life has been busy!

I should probably update to use the latest version of Howler at some point (it's not an intentional thing, it's just that 2.1.2 was the latest version when this package was first released). Or, I should probably make it a peer dependency now that NPM complains about missing peer deps 🤔

These are all challenges for another day, and honestly I'm not deep enough in the TS ecosystem to know what the best practices are. So I'll get this merged so that folks are unblocked, and possibly switch to the official types at some point down the line.

Thanks for this contribution, @TamoshaytisV!

joshwcomeau commented 3 years ago

Published in 2.1.0

TimAstier commented 3 years ago

Tested in my app and it works 🚀 thank you @TamoshaytisV @joshwcomeau, looks much cleaner now :) Link to the satisfying git diff: https://github.com/TimAstier/react-rpg-game/commit/66856b83ed079334dc5231816a6a7af28bfd553b