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

WOLFLEADER•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

WOLFLEADER•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

WOLFLEADER•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

WOLFLEADER•2y ago

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

Brendonovich•2y ago

yeah fairo

WOLFLEADER•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

WOLFLEADER•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

WOLFLEADER•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

Gaming

Programming

UseEffect not being called