joshwcomeau / use-sound

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

Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function #14

Closed azz0r closed 4 years ago

azz0r commented 4 years ago

Hey interesting issue - I'm redirecting a user after a sound and it looks like potentially a useEffect within use-sound isn't returning a way to unmount. Thoughts?

  const [play,] = useSound(beepSfx)
  const history = useHistory()

  const onBeep = useCallback(() => play())

  const onCreate = useCallback(() => {
    onBeep()
    history.push(ROUTES.MANAGE_BRANDS)
  })

Console errors:

index.js:1 Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.
    in WelcomeSelectPage (created by Context.Consumer)
  | console.<computed> | @ | index.js:1
-- | -- | -- | --
  | r | @ | react_devtools_backend.js:6
  | printWarning | @ | react-dom.development.js:88
  | error | @ | react-dom.development.js:60
  | warnAboutUpdateOnUnmountedFiberInDEV | @ | react-dom.development.js:23192
  | scheduleUpdateOnFiber | @ | react-dom.development.js:21200
  | dispatchAction | @ | react-dom.development.js:15682
  | (anonymous) | @ | index.ts:105
  | (anonymous) | @ | howler.js:1856
  | setTimeout (async) |   |  
  | _emit | @ | howler.js:1855
  | _ended | @ | howler.js:1920
  | setTimeout (async) |   |  
  | playWebAudio | @ | howler.js:843
  | play | @ | howler.js:855
  | (anonymous) | @ | index.ts:103
  | (anonymous) | @ | select.js:37

Line 37 is onBeep declaration

joshwcomeau commented 4 years ago

Hey @azz0r,

This was just fixed in a recent PR :) about to publish 1.0.2. Shouldn't happen any more!

ludgerey commented 4 years ago

I have the same issue with useEffect. I am already on version 1.0.2.

I'm building a countdown with audio signals.

    const [timeLeft, setTimeLeft] = useState(10);
    const [playCountdownTick] = useSound(countdownTickSound);
    const [playCountdownComplete] = useSound(countdownCompleteSound);

    useEffect(() => {
        if (paused) {
            return undefined;
        }

        if (!timeLeft) {
            playCountdownComplete();
            onComplete();
            return undefined;
        }

        if (timeLeft <= 5) {
            playCountdownTick();
        }

        const intervalId = setInterval(() => {
            setTimeLeft(timeLeft - 1);
        }, 1000);

        // clean up after firing effect
        return () => clearInterval(intervalId);
    }, [timeLeft, setTimeLeft, paused, onComplete, playCountdownTick, playCountdownComplete]);

Is there anything I do wrong? By the way I tried to stop the sounds during clean up, but didn't help. The component is removed after the countdown is finished and then I get the error:

Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.

Update:

Ok. Stopping actually works if I use a separate useEffect block. But then again the sound is not played at the end of the countdown. Probably have to find a completely different solution.

useEffect(() => {
        if (!timeLeft) {
            playCountdownComplete();
        }

        if (timeLeft <= 5) {
            playCountdownTick();
        }

        // clean up after firing effect
        return () => {
            stopCountdownTick();
            stopCountdownComplete();
        };
    }, [timeLeft, playCountdownTick, playCountdownComplete, stopCountdownTick, stopCountdownComplete]);

Update 2:

I moved the playCountdownComplete sound to the next screen. I got my problem solved ;)

ghost commented 4 years ago

@joshwcomeau actually got this warning as well, it occurs after navigating to another page in Gatsby. Any chance you can re-open this?

const Button = props => {
  const [play] = useSound(bubble, { volume: 1 })

  const executeClick = () => {
    play()
    props.onClick && props.onClick()
  }

  return (
    <ButtonWrapper props={props} onClick={executeClick}>
      {props.children}
    </ButtonWrapper>
  )
}
index.js:2177 Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.
    in Button (at pages/index.js:229)
    in a (created by Context.Consumer)
    in Location (created by Context.Consumer)
    in Link (created by Context.Consumer)
    in Location (created by GatsbyLink)
    in GatsbyLink (created by ForwardRef)
    in ForwardRef (at pages/index.js:228)
    in div (at pages/index.js:201)
    in main (at pageWrapper.js:112)
    in div (created by Context.Consumer)
    in StyledComponent (created by pageWrapper__InnerWrapper)
    in pageWrapper__InnerWrapper (at pageWrapper.js:110)
    in div (created by Context.Consumer)
    in StyledComponent (created by pageWrapper__Wrapper)
    in pageWrapper__Wrapper (at pageWrapper.js:109)
    in PageWrapper (created by ConnectFunction)
    in ConnectFunction (at pages/index.js:153)
    in IndexPage (created by ConnectFunction)
    in ConnectFunction (created by HotExportedConnect(IndexPage))
    in AppContainer (created by HotExportedConnect(IndexPage))
    in HotExportedConnect(IndexPage) (created by PageRenderer)
alatarus commented 4 years ago

This issue still occurs because the play function changes state asynchronously.

curt-tophatter commented 4 years ago

Great catch @alatarus, this does seem like the root of the issue. I'm experiencing this on v1.0.2 as well. @joshwcomeau could we get this issue reopened?

curt-tophatter commented 4 years ago

Actually it looks like this line is causing this error for me:

Screen Shot 2020-11-21 at 12 32 33 PM
joshwcomeau commented 4 years ago

Good catch :+1:

My hunch is we'd have to solve this with an isMounted ref, and avoid the calls to setDuration and setIsPlaying if it's no longer true.

I'm happy to reopen the issue, but I'm afraid that's about all I can do right now; I have a limited amount of typing ability in a given day, due to a medical issue. If anyone wants to open a PR I'm open to that! Though I can't promise a speedy review.

andrekat commented 3 years ago

still happening in latest version - any suggestions?

useEffect(() => {

        if (spinWheel.spin && spinWheel.number >= 0) {
            startSpinningSound()
        }

        return () => {
            stop()
        }

    }, [spinWheel, stop])
index.js:1 Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.
    in Wheel (at Play.tsx:254)
console.<computed> @ index.js:1
overrideMethod @ react_devtools_backend.js:4049
printWarning @ react-dom.development.js:88
error @ react-dom.development.js:60
warnAboutUpdateOnUnmountedFiberInDEV @ react-dom.development.js:23161
scheduleUpdateOnFiber @ react-dom.development.js:21169
dispatchAction @ react-dom.development.js:15660
handleLoad @ index.ts:34
(anonymous) @ howler.js:1903
setTimeout (async)
_emit @ howler.js:1902
loadSound @ howler.js:2500
success @ howler.js:2467
Promise.then (async)
decodeAudioData @ howler.js:2475
xhr.onload @ howler.js:2424
load (async)
loadBuffer @ howler.js:2416
load @ howler.js:729
init @ howler.js:646
(anonymous) @ howler.js:2751
Howl @ howler.js:565
(anonymous) @ index.ts:51
Promise.then (async)
(anonymous) @ index.ts:43
commitHookEffectListMount @ react-dom.development.js:19731
commitPassiveHookEffects @ react-dom.development.js:19769
callCallback @ react-dom.development.js:188
invokeGuardedCallbackDev @ react-dom.development.js:237
invokeGuardedCallback @ react-dom.development.js:292
flushPassiveEffectsImpl @ react-dom.development.js:22853
unstable_runWithPriority @ scheduler.development.js:653
runWithPriority$1 @ react-dom.development.js:11039
flushPassiveEffects @ react-dom.development.js:22820
performSyncWorkOnRoot @ react-dom.development.js:21737
(anonymous) @ react-dom.development.js:11089
unstable_runWithPriority @ scheduler.development.js:653
runWithPriority$1 @ react-dom.development.js:11039
flushSyncCallbackQueueImpl @ react-dom.development.js:11084
flushSyncCallbackQueue @ react-dom.development.js:11072
discreteUpdates$1 @ react-dom.development.js:21893
discreteUpdates @ react-dom.development.js:806
dispatchDiscreteEvent @ react-dom.development.js:4168
AdrienLemaire commented 2 years ago

I'm also seeing this issue in v4.0.1 with react v17.0.2 . Calling navigate or returning a Navigate component from react-router-dom at the first render after an authorization check will throw the warning

@joshwcomeau any idea what this could be ?

VadimCiv commented 2 years ago

I have the same problem here

const [play, { stop }] = useSound(alertSfx);

Screen Shot 2022-02-13 at 3 52 46 PM Screen Shot 2022-02-13 at 3 52 04 PM

const alertsCritical = useSelector((state) => state.alerts.alertsCritical, shallowEqual);

useEffect(() => { if (alertsCritical.length > 0) { play(); } return () => { if (alertsCritical.length > 0) { stop(); } }; }, [alertsCritical, play, stop]);

natoszme commented 2 years ago

Same problem here (4.0.1) @meowsus I'm getting the warning when my component is unmounted, and only by invoking the hook:

const [playOkSound] = useSound(okSound);

(the use of playOkSound is commented and I'm still getting it)

Thanks!