1

I have a useEffect() that checks a trigger boolean in state if there is a message sound that should play, and after playing it sets that active message trigger to false in state.

However, the useEffect() goes into an infinite loop crashing the app. Probably because changing the state triggers it again (and again...)

Usually, with useState, this is fairly simple to fix with something like useEffect(() => {logic}, [trigger])

In my case I am not using useState, but I am using a reducer to change state.

Edit: The weird thing is, the reducer sometimes works to modify state, and sometimes it does not. It will execute without errors but the state remains unchanged.

Let me show you my commented code:

import React, { useEffect } from "react";
import { getCachedImage } from "../helpers";

const MessageNotification = (props) => {
  const messageImg= getCachedImage("/ui/newmessage.png");

  

  // Function that plays given sound
  function playSound(soundFile) {
    let audio = new Audio("/audio/messages/" + soundFile);
    audio.play();
  }

  // Check if a Message is set to active. If so, execute logic
  useEffect(() => {
    // Get messages from state and find the message with "active" set to true
    const messagesState = props.state.messages;
    const activeMessage = messagesState.find((element) => element.active === true);

    if (activeMessage) {

      playSound(activeMessage.audio);

      // Mark the message as userNotified and set it to inactive in state
      let updatedMessagesState = messagesState ;
      let index = messagesState.indexOf(activeMessage);

      if (~index) {
        updatedMessagesState[index].userNotified= true;
        updatedMessagesState[index].active = false;
      }

      /* This is the weird part, the updatedMessagesState is correct, 
but the dispatch reducer does not pass it to state. 
This does work when I remove the useEffect 
(but that gives me a fat red warning in console) */

      props.dispatch({ type: "UPDATE_MESSAGES", payload: updatedMessagesState });
    }
  });

  return (
    <div>
      <img src={"images" + messageImg} alt="message" width="90" height="90"></img>
    </div>
  );
};

export default MessageNotification;

As you can see, I do not use useState but work with a reducer instead. The solution that I often find which pertains to something like the following is not my solution as far as I can tell:

// Not applicable solution for me, since I use reducer
const [trigger] = useState();

useEffect(() => {
    // Logic here
}, [trigger]);

Edit: Since the reducer does not seem to modify state when used in useEffect, let me post its code:

const reducer = (state, action) => {
  switch (action.type) {
    case "UPDATE_MESSAGES":
      return { ...state, messages: action.payload };
    default:
      throw new Error();
  }
};

export default reducer;
Ozone
  • 1,337
  • 9
  • 18

3 Answers3

2

Try adding a dependency for your useEffect, such as:

useEffect(() => {
    if (activeMessage) {

      playSound(activeMessage.audio);

      //mark the message as userNotified and set it to inactive in state
      let updatedMessagesState = messagesState ;
      let index = messagesState.indexOf(activeMessage);

      if (~index) {
        updatedMessagesState[index].userNotified= true;
        updatedMessagesState[index].active = false;
      }

      props.dispatch({ type: "UPDATE_MESSAGES", payload: updatedMessagesState });
    }
  }, [activeMessage]);

By not specifying a dependency array, your useEffect will run on EVERY render, hence creating an infinite loop.

Also, you are trying to directly modify a prop (and it is an anti pattern) on this line:

const messagesState = props.state.messages;

Try changing it to this:

const messagesState = [...props.state.messages];

Also, let index = messagesState.indexOf(activeMessage); will not work since messagesState is an object array. To get the index of the active message, try this:

let index = messagesState.map(message => message.active).indexOf(true);
Sagi Rika
  • 2,839
  • 1
  • 12
  • 32
  • Thanks for your post. I just tried doing this, even with single AND multiple dependency array values but the error remains. Let me step debug it and update my post – Ozone Oct 05 '20 at 11:23
  • I've applied your edits as well, but the issue seems to be that the reducer does not modify state when within a useEffect, which is weird cause it does work when I remove the useEffect. See Q edit. – Ozone Oct 05 '20 at 12:04
  • 1
    It worked! Your last edit in combination with making it a const defined outside the useEffect worked. – Ozone Oct 05 '20 at 13:10
1

I think if you add props.state.messages as dependency, the problem will fixed. Also if you use only the messagesState and messagesState in useEffect, you should move this variables to that block:

 useEffect(() => {
    const messagesState = props.state.messages;
    const messagesState = messagesState.find((element) => element.active === true);

    if (activeMessage) {

      playSound(activeMessage.audio);

      //mark the message as userNotified and set it to inactive in state
      let updatedMessagesState = messagesState ;
      let index = messagesState.indexOf(activeMessage);

      if (~index) {
        updatedMessagesState[index].userNotified= true;
        updatedMessagesState[index].active = false;
      }

      /* This is the weird part, the updatedMessagesState is correct, 
but the dispatch reducer does not pass it to state. 
This does work when I remove the useEffect 
(but that gives me a fat red warning in console) */

      props.dispatch({ type: "UPDATE_MESSAGES", payload: updatedMessagesState });
    }
  }, [props.state.messages]);
lissettdm
  • 12,267
  • 1
  • 18
  • 39
  • Thanks for your post! Unfortunately, that does not help. Neither does placing other vars or even props in there. – Ozone Oct 05 '20 at 12:52
  • Maybe the solution is to move this logic to the parent component where the reduce state and dispatch action are declared. At the moment that message.active changes, you should dispatch UPDATE_MESSAGES action. How are you implementing the parent component? – lissettdm Oct 05 '20 at 13:09
1
// Check if a Message is set to active. If so, execute logic
  useEffect(() => {
    // Get messages from state and find the message with "active" set to true
    const messagesState = props.state.messages;
    const activeMessage = messagesState.find((element) => element.active === true);

    if (activeMessage) {

      playSound(activeMessage.audio);

      // Mark the message as userNotified and set it to inactive in state
      let updatedMessagesState = messagesState ;
      let index = messagesState.indexOf(activeMessage);

      if (~index) {
        updatedMessagesState[index].userNotified= true;
        updatedMessagesState[index].active = false;
      }

      /* This is the weird part, the updatedMessagesState is correct, 
but the dispatch reducer does not pass it to state. 
This does work when I remove the useEffect 
(but that gives me a fat red warning in console) */

      props.dispatch({ type: "UPDATE_MESSAGES", payload: updatedMessagesState });
    }
  });

your useEffect needs a dependency, if you are not providing dependency in useEffect like in your case it'll always run on every render. Provide [] as second argument in your useEffect or [any state or prop on which this effect depends].

Piyush Rana
  • 631
  • 5
  • 8