UseEffect not being called

Hey, I've got a weird bug in my react component

const MqttProvider = ({ children, options }: Props) => {
  const [mqtt, setMqtt] = useState<MqttClient>();
  const [connected, setConnected] = useState(false);

  const onConnect = () => {
    setConnected(true);
  };

  const onClose = () => {
    setConnected(false);
  };

  const initMqtt = async () => {
    try {
      const broker = await getBroker();

      const newMqtt = connect(broker, options);
      newMqtt.setMaxListeners(0);
      newMqtt.on("connect", onConnect);
      newMqtt.on("close", onClose);
      newMqtt.on("error", onClose);
      setMqtt(newMqtt);
    } catch (e) {
      console.error(e);
    }
  };

  useEffect(() => {
    initMqtt();
  }, []);

  useEffect(() => {
    console.log(mqtt?.connected);
  }, [mqtt?.connected]);

  return (
    <MqttContext.Provider value={{ mqtt }}>{children}</MqttContext.Provider>
  );
};

const MqttProvider = ({ children, options }: Props) => {
  const [mqtt, setMqtt] = useState<MqttClient>();
  const [connected, setConnected] = useState(false);

  const onConnect = () => {
    setConnected(true);
  };

  const onClose = () => {
    setConnected(false);
  };

  const initMqtt = async () => {
    try {
      const broker = await getBroker();

      const newMqtt = connect(broker, options);
      newMqtt.setMaxListeners(0);
      newMqtt.on("connect", onConnect);
      newMqtt.on("close", onClose);
      newMqtt.on("error", onClose);
      setMqtt(newMqtt);
    } catch (e) {
      console.error(e);
    }
  };

  useEffect(() => {
    initMqtt();
  }, []);

  useEffect(() => {
    console.log(mqtt?.connected);
  }, [mqtt?.connected]);

  return (
    <MqttContext.Provider value={{ mqtt }}>{children}</MqttContext.Provider>
  );
};

For some reason, when mqtt.connected is changed the useEffect is not called. For example if i start the broker, it console logs false and then when i disconnect it console logs true its almost as if the useEffect is reading the previous state. However if i change the depedency to be my connected state variable instead, it updates perfectly.... ( was previously wrapping the return in react.memo but i have removed it for this example, as it presents the same issue, the depedency of mqtt?.connected doesnt get called)

18 Replies

Brendonovich•2y ago

the issue is that when mqtt.connected is changed, it's not causing the value of mqtt itself to change, since setMqtt isn't being called with a new value. setConnected is updating the connected state, so it rerenders and triggers the useEffect

WOLFLEADEROP•2y ago

right so what would be the best way around this? continue to just store my own internal connected state (which is kinda of annoying , and makes me have to listen to events)

Brendonovich•2y ago

yeah you'd need to maintain your own state, since u cant just make an immutable copy of the mqtt broker It's a good candidate for a custom hook actually

WOLFLEADEROP•2y ago

hmmmm yea good point yea coz mqtt is immuntable, so me changing it doesnt actually work... skipped my mind when i wrote tthis lol what sort of hook? I can't think of any way for the hook to work?

Brendonovich•2y ago

something like this

export const useMqtt = (options) => {
  // useRef since the client isn't regular state
  const client = useRef<MqttClient | undefined>();

  const [state, setState] = useState({
    connected: false,
    // whatever other client state u have
  });

  const onConnect = () => {
    setState((state) => ({ ...state, connected: true }));
  };

  const onClose = () => {
    setState((state) => ({ ...state, connected: false }));
  };

  const initMqtt = async () => {
    try {
      const broker = await getBroker();

      const newMqtt = connect(broker, options);
      newMqtt.setMaxListeners(0);
      newMqtt.on("connect", onConnect);
      newMqtt.on("close", onClose);
      newMqtt.on("error", onClose);
      client.current = newMqtt;
    } catch (e) {
      console.error(e);
    }
  };

  return {
    ...state,
    client: client.current,
  };
};

export const useMqtt = (options) => {
  // useRef since the client isn't regular state
  const client = useRef<MqttClient | undefined>();

  const [state, setState] = useState({
    connected: false,
    // whatever other client state u have
  });

  const onConnect = () => {
    setState((state) => ({ ...state, connected: true }));
  };

  const onClose = () => {
    setState((state) => ({ ...state, connected: false }));
  };

  const initMqtt = async () => {
    try {
      const broker = await getBroker();

      const newMqtt = connect(broker, options);
      newMqtt.setMaxListeners(0);
      newMqtt.on("connect", onConnect);
      newMqtt.on("close", onClose);
      newMqtt.on("error", onClose);
      client.current = newMqtt;
    } catch (e) {
      console.error(e);
    }
  };

  return {
    ...state,
    client: client.current,
  };
};

you give it options, it returns a client and state const { client, connected } = useMqtt(...) wait i didn't update it enough

WOLFLEADEROP•2y ago

xd all good i get the concept, its very similar to what i already had yea

Brendonovich•2y ago

yeah it's just putting the client logic and state in a little helper

WOLFLEADEROP•2y ago

yea i might just use that directly in the provider instead of a seperate hook.

Brendonovich•2y ago

yeah fairo

WOLFLEADEROP•2y ago

would be nice to be able to access client.connected without having to export my own state but yea it works

Brendonovich•2y ago

yeah since the client isn't designed for react and immutability it won't work

WOLFLEADEROP•2y ago

yea and coz even if i change it to a ref, it wont work in use effects etc

Brendonovich•2y ago

i think the proper way to do this is useSyncExternalStore, but you're probs just building an app yeah using a ref doesn't change much, it's just a bit more correct and communicates the fact that the client won't cause rerenders or be updated with a setter function

WOLFLEADEROP•2y ago

yea ill look into that i guess i could hook it up like this

    <MqttContext.Provider value={React.useMemo(() => ({ mqtt: client.current }), [state])}>
      {children}
    </MqttContext.Provider>

    <MqttContext.Provider value={React.useMemo(() => ({ mqtt: client.current }), [state])}>
      {children}
    </MqttContext.Provider>

that way it will update when state changes and calling mqtt.connected should always be correct

Brendonovich•2y ago

hmmm i guess but i think relying on the client itself to have the connected state be up to date isn't reliable

WOLFLEADEROP•2y ago

oh wait this causes infinte re renders xd

Brendonovich•2y ago

without a dedicated useState, you simply won't get rerenders when the client's state changes may as well just use the state

WOLFLEADEROP•2y ago

yea for some reason its ifintiely re rendering right now

const MqttProvider = ({ children, options }: Props) => {
  const client = useRef<MqttClient | undefined>();

  const [state, setState] = useState({
    connected: false,
    // whatever other client state u have
  });

  const onConnect = () => {
    setState((state) => ({ ...state, connected: true }));
  };

  const onClose = () => {
    setState((state) => ({ ...state, connected: false }));
  };

  const initMqtt = async () => {
    try {
      const broker = await getBroker();

      const newClient = connect(broker, options);
      newClient.setMaxListeners(0);
      newClient.on("connect", onConnect);
      newClient.on("close", onClose);
      newClient.on("error", onClose);

      client.current = newClient;
    } catch (e) {
      console.error(e);
    }
  };

  useEffect(() => {
    initMqtt();
  }, []);

  useEffect(() => {
    console.log("*************");
    console.log(client.current?.connected);
  }, [state.connected]);

  return (
    <MqttContext.Provider
      value={React.useMemo(() => ({ mqtt: client.current, ...state }), [state])}
    >
      {children}
    </MqttContext.Provider>
  );
};

const MqttProvider = ({ children, options }: Props) => {
  const client = useRef<MqttClient | undefined>();

  const [state, setState] = useState({
    connected: false,
    // whatever other client state u have
  });

  const onConnect = () => {
    setState((state) => ({ ...state, connected: true }));
  };

  const onClose = () => {
    setState((state) => ({ ...state, connected: false }));
  };

  const initMqtt = async () => {
    try {
      const broker = await getBroker();

      const newClient = connect(broker, options);
      newClient.setMaxListeners(0);
      newClient.on("connect", onConnect);
      newClient.on("close", onClose);
      newClient.on("error", onClose);

      client.current = newClient;
    } catch (e) {
      console.error(e);
    }
  };

  useEffect(() => {
    initMqtt();
  }, []);

  useEffect(() => {
    console.log("*************");
    console.log(client.current?.connected);
  }, [state.connected]);

  return (
    <MqttContext.Provider
      value={React.useMemo(() => ({ mqtt: client.current, ...state }), [state])}
    >
      {children}
    </MqttContext.Provider>
  );
};

only reason i know this is because i have whydidyourerender plugin hmm seems like when the broker is disconnected it continously calls onClose

 LOG    {"PressureGauge": [Function PressureGauge]} Re-rendered because of hook changes:
 GROUP      [hook useContext result]
 LOG      different objects that are equal by value. (more info at http://bit.ly/wdyr3)
 LOG      {"prev ": {"connected": true, "mqtt": {"_events": [Object], "_eventsCount": 3, "_firstConnection": false, "_maxListeners": 0, "_packetIdsDuringStoreProcessing": [Object], "_resubscribeTopics": [Object], "_storeProcessing": false, "connackTimer": 29725, "connected": true, "disconnecting": false, "incomingStore": [Store], "messageIdToTopic": [Object], "nextId": 64719, "options": [Object], "outgoing": [Object], "outgoingStore": [Store], "pingResp": true, "pingTimer": [ReInterval], "queue": [Array], "queueQoSZero": false, "reconnectTimer": null, "reconnecting": false, "stream": [Duplexify], "streamBuilder": [Function wrapper]}}} !== {"next ": {"connected": true, "mqtt": {"_events": [Object], "_eventsCount": 3, "_firstConnection": false, "_maxListeners": 0, "_packetIdsDuringStoreProcessing": [Object], "_resubscribeTopics": [Object], "_storeProcessing": false, "connackTimer": 29725, "connected": true, "disconnecting": false, "incomingStore": [Store], "messageIdToTopic": [Object], "nextId": 64719, "options": [Object], "outgoing": [Object], "outgoingStore": [Store], "pingResp": true, "pingTimer": [ReInterval], "queue": [Array], "queueQoSZero": false, "reconnectTimer": null, "reconnecting": false, "stream": [Duplexify], "streamBuilder": [Function wrapper]}}}

 LOG    {"PressureGauge": [Function PressureGauge]} Re-rendered because of hook changes:
 GROUP      [hook useContext result]
 LOG      different objects that are equal by value. (more info at http://bit.ly/wdyr3)
 LOG      {"prev ": {"connected": true, "mqtt": {"_events": [Object], "_eventsCount": 3, "_firstConnection": false, "_maxListeners": 0, "_packetIdsDuringStoreProcessing": [Object], "_resubscribeTopics": [Object], "_storeProcessing": false, "connackTimer": 29725, "connected": true, "disconnecting": false, "incomingStore": [Store], "messageIdToTopic": [Object], "nextId": 64719, "options": [Object], "outgoing": [Object], "outgoingStore": [Store], "pingResp": true, "pingTimer": [ReInterval], "queue": [Array], "queueQoSZero": false, "reconnectTimer": null, "reconnecting": false, "stream": [Duplexify], "streamBuilder": [Function wrapper]}}} !== {"next ": {"connected": true, "mqtt": {"_events": [Object], "_eventsCount": 3, "_firstConnection": false, "_maxListeners": 0, "_packetIdsDuringStoreProcessing": [Object], "_resubscribeTopics": [Object], "_storeProcessing": false, "connackTimer": 29725, "connected": true, "disconnecting": false, "incomingStore": [Store], "messageIdToTopic": [Object], "nextId": 64719, "options": [Object], "outgoing": [Object], "outgoingStore": [Store], "pingResp": true, "pingTimer": [ReInterval], "queue": [Array], "queueQoSZero": false, "reconnectTimer": null, "reconnecting": false, "stream": [Duplexify], "streamBuilder": [Function wrapper]}}}

hmmm uhh the state is an object thats why altho on state change it stil causes that re render which isnt ideal (just doesnt spam it anymore)

Gaming

Programming

UseEffect not being called